Small TODO refactoring

vizro-core/examples/_dev/app.py

@@ -1,5 +1,6 @@
 """Rough example used by developers."""
-from dash import Output
+from dash import Output, State
+from dash import Output, State
 
 import vizro.models as vm
 import vizro.plotly.express as px
@@ -16,58 +17,59 @@
 
 
 # Just a util function
-def _build_card_text_message(card_cellClicked: Optional[Dict] = None):
-    if card_cellClicked:
+def _build_card_text_message(grid_cell_clicked: Optional[Dict] = None):
+    if grid_cell_clicked:
         return f"""
             "Scatter" plot filtered by:
 
-            column: **{card_cellClicked.get('colId')}**
+            column: **{grid_cell_clicked.get('colId')}**
 
-            value: "**{card_cellClicked.get('value')}**"
+            value: "**{grid_cell_clicked.get('value')}**"
          """
     return default_card_text
 
 
 # Pure custom action to update card text with grid clicked data.
 @capture("action")
-def update_card_with_grid_clicked_data(card_cellClicked: Optional[Dict] = None):
-    return _build_card_text_message(card_cellClicked=card_cellClicked)
+def update_card_with_grid_clicked_data(grid_cell_clicked: Optional[Dict] = None):
+    return _build_card_text_message(grid_cell_clicked=grid_cell_clicked)
 
 
 # 1. Overwriting the filter_interaction action by overwriting only the pure_function
 # TODO-AV2-OQ: It doesn't seem possible to achieve this, since _action._get_callback_mapping can't reconcile
-#   self.inputs (List[States]) and self.function.inputs (Dict[str, List[State]]) at the same time.
+#  self.inputs (List[States]) and self.function.inputs (Dict[str, List[State]]) at the same time. There is many ways
+#  to solve this, but it's not clear which one is the best.
 @capture("action")
 def overwritten_filter_interactions_1(
     targets: Optional[List[ModelID]] = None,
-    card_cellClicked: Optional[Dict] = None,
+    grid_cell_clicked: Optional[Dict] = None,
     **inputs: Dict[str, Any]
 ):
     return {
-        "card.children": _build_card_text_message(card_cellClicked=card_cellClicked),
+        "card_children": _build_card_text_message(grid_cell_clicked=grid_cell_clicked),
         **filter_interaction.pure_function(targets=targets, **inputs),
     }
 
-# 2. Overwriting the filter_interaction action by inheriting FilterInteractionAction
-from vizro.actions.filter_interaction_action import FilterInteractionAction
-from dash import Output, State
+# 2. Overwriting the filter_interaction action by inheriting filter_interaction
 
-
-class OverwrittenFilterInteractions2(FilterInteractionAction):
+# This also works:
+# from vizro.actions.filter_interaction_action import FilterInteractionAction
+# class OverwrittenFilterInteractions2(FilterInteractionAction):
+class OverwrittenFilterInteractions2(filter_interaction):
     @staticmethod
-    def pure_function(targets: list = None, card_cell_clicked: dict = None, **kwargs):
+    def pure_function(targets: list = None, grid_cell_clicked: dict = None, **kwargs):
         return {
-            "card_children": _build_card_text_message(card_cellClicked=card_cell_clicked),
+            "card_children": _build_card_text_message(grid_cell_clicked=grid_cell_clicked),
             # TODO-AV2-OQ: Is it possible to get rid of 'targets=targets' (and potentially **kwargs) since targets
-            #   should already be attached (added as partial arguments) within
-            #   super()._post_init (self._arguments["targets"] = targets)?
-            **FilterInteractionAction.pure_function(targets=targets, **kwargs),
+            #  should already be attached (added as partial arguments) within
+            #  super()._post_init (self._arguments["targets"] = targets)?
+            **filter_interaction.pure_function(targets=targets, **kwargs),
         }
 
     @property
     def inputs(self):
         return {
-            "card_cell_clicked": State("underlying_ag_grid", "cellClicked"),
+            "grid_cell_clicked": State("underlying_ag_grid", "cellClicked"),
             **super().inputs,
         }
 
@@ -98,16 +100,16 @@ def outputs(self):
                         #     function=filter_interaction(
                         #         targets=[
                         #             "scatter",
-                        #             # "scatter_from_page_2",
-                        #         ]
-                        #     )
+                        #             "scatter_from_page_2",
+                                # ]
+                            # )
                         # ),
                         # Implementing updating card with grid clicked data as the independent action.
-                        vm.Action(
-                            function=update_card_with_grid_clicked_data(),
-                            inputs=["underlying_ag_grid.cellClicked"],
-                            outputs=["card.children"],
-                        ),
+                        # vm.Action(
+                        #     function=update_card_with_grid_clicked_data(),
+                        #     inputs=["underlying_ag_grid.cellClicked"],
+                        #     outputs=["card.children"],
+                        # ),
                         # Now, let's try to implement updating card text by overwriting the filter_interaction action
                         # 1. Overwriting the filter_interaction action by overwriting only the pure_function
                         # vm.Action(
@@ -146,12 +148,13 @@ def outputs(self):
                 ),
             ],
             controls=[
-                # TODO-AV2-TICKET: 1. overwrite filter function so it cancels the card.children changes too.
-                #   2. OR attach the 'update_card_with_grid_clicked_data' action after the default filter action.
+                # TODO-AV2-TICKET-NEW: Overwrite filter function so it cancels the card.children changes too.
+                # TODO-AV2-TICKET-CREATED: Attach the 'update_card_with_grid_clicked_data' action after
+                #  the default filter action.
                 vm.Filter(targets=["scatter"], column="continent"),
                 vm.Parameter(targets=["scatter.x"], selector=vm.RadioItems(options=["gdpPercap", "lifeExp"]))
             ],
-            # TODO-AV2-TICKET: Try to attach the 'update_card_with_grid_clicked_data' action after update_figures action.
+            # TODO-AV2-TICKET-CREATED: Try to attach the custom action that clicks data after the update_figures action.
         ),
         vm.Page(
             title="Page 2",

vizro-core/src/vizro/actions/_actions_utils.py

@@ -54,16 +54,8 @@ def _apply_filters(data_frame: pd.DataFrame, ctds_filters: List[CallbackTriggerD
         selector_actions = _get_component_actions(model_manager[ctd["id"]])
 
         for action in selector_actions:
-            if (
-                # TODO-AV2: Check do we need to check the statement given in the comment below.
-                # We don't need to check if the function name is _filter anymore, since every action is a filter now.
-                # action.function._function.__name__ != "_filter"
-                # or target not in action.function["targets"]
-
             # TODO-AV2: Handle if "action.function != "filter_action" until inputs refactoring
-            target not in action.function.targets
-                or ALL_OPTION in selector_value
-            ):
+            if (target not in action.function.targets or ALL_OPTION in selector_value):
                 continue
 
             _filter_function = action.function["filter_function"]
@@ -138,7 +130,7 @@ def _get_parametrized_config(
 ) -> Dict[ModelID, Dict[str, Any]]:
     parameterized_config = {}
     for target in targets:
-        # TODO-AV2 - avoid calling _captured_callable. Once we have done this we can remove _arguments from
+        # TODO-AV2 - avoid calling _captured_callable. Once we have done this and similar we can remove _arguments from
         #  CapturedCallable entirely.
         graph_config = deepcopy(model_manager[target].figure._arguments)
         if "data_frame" in graph_config:

vizro-core/src/vizro/actions/_callback_mapping/_callback_mapping_utils.py

@@ -25,8 +25,10 @@ def _get_matching_page_actions_by_action_class(
     ]
 
 
-# TODO-AV2-TICKET: Once "actions_info" is implemented, functions like:
+# TODO-AV2-TICKET-CREATED: Once "actions_info" is implemented, functions like:
 #  _get_inputs_of_filters, _get_inputs_of_parameters, _get_inputs_of_figure_interactions will become a single function.
+#  This approach should enable predefined actions to see custom actions if they are inherited from the predefined
+#  actions (which is a use case we want to achieve).
 def _get_inputs_of_filters(
     page: Page,
     action_class: CapturedActionCallable = None
@@ -36,7 +38,6 @@ def _get_inputs_of_filters(
         page_id=ModelID(str(page.id)), action_class=action_class
     )
     inputs = []
-    # TODO-AV2-TICKET: Take the "actions_info" into account once it's implemented.
     for action in filter_actions_on_page:
         triggered_model = model_manager._get_action_trigger(action_id=ModelID(str(action.id)))
         inputs.append(State(component_id=triggered_model.id, component_property=triggered_model._input_property))
@@ -53,7 +54,6 @@ def _get_inputs_of_parameters(
         page_id=ModelID(str(page.id)), action_class=action_class
     )
     inputs = []
-    # TODO-AV2-TICKET: Take the "actions_info" into account once it's implemented.
     for action in parameter_actions_on_page:
         triggered_model = model_manager._get_action_trigger(action_id=ModelID(str(action.id)))
         inputs.append(State(component_id=triggered_model.id, component_property=triggered_model._input_property))
@@ -64,12 +64,11 @@ def _get_inputs_of_parameters(
 def _get_inputs_of_figure_interactions(
     page: Page, action_class: CapturedActionCallable = None
 ) -> List[Dict[str, State]]:
-    """Gets list of `States` for selected chart interaction `action_function` of triggered `Page`."""
+    """Gets list of `States` for all components that have the `filter_interaction` action from the `Page`."""
     figure_interactions_on_page = _get_matching_page_actions_by_action_class(
         page_id=ModelID(str(page.id)), action_class=action_class
     )
     inputs = []
-    # TODO-AV2-TICKET: Take the "actions_info" into account once it's implemented.
     for action in figure_interactions_on_page:
         triggered_model = model_manager._get_action_trigger(action_id=ModelID(str(action.id)))
         required_attributes = ["_filter_interaction_input", "_filter_interaction"]

vizro-core/src/vizro/actions/export_data_action.py

@@ -19,13 +19,13 @@ def _post_init(self):
         self._page_id = model_manager._get_model_page_id(model_id=self._action_id)
 
         # Validate and calculate "targets"
-        # TODO-AV2-EASY: Make targets validation reusable for the other actions too.
+        # TODO-AV2-TICKET-NEW: Make targets validation reusable for the other actions too.
         # TODO-AV2-OQ: Rethink using self._arguments
         targets = self._arguments.get("targets")
         if targets:
             for target in targets:
                 if self._page_id != model_manager._get_model_page_id(model_id=target):
-                    # TODO-AV2-EASY: Improve error message to explain in which action the error occurs.
+                    # TODO-AV2-TICKET-NEW: Improve error message to explain in which action the error occurs.
                     raise ValueError(f"Component '{target}' does not exist on the page '{self._page_id}'.")
         else:
             targets = model_manager._get_page_model_ids_with_figure(page_id=self._page_id)
@@ -85,6 +85,7 @@ def pure_function(
 
     @property
     def inputs(self):
+        # TODO-AV2: Consider using aliases like """from vizro.actions import filter_action, filter_interaction""" below.
         from vizro.actions._callback_mapping._callback_mapping_utils import (
             _get_inputs_of_figure_interactions,
             _get_inputs_of_filters,
@@ -102,7 +103,7 @@ def inputs(self):
 
     @property
     def outputs(self) -> Dict[str, Output]:
-        # TODO-AV2-TICKET: Take the "actions_info" into account once it's implemented.
+        # TODO-AV2-TICKET-CREATED: Take the "actions_info" into account once it's implemented.
         return {
             f"download_dataframe_{target}": Output(
                 component_id={"type": "download_dataframe", "action_id": self._action_id, "target_id": target},
@@ -113,7 +114,7 @@ def outputs(self) -> Dict[str, Output]:
 
     @property
     def components(self):
-        # TODO-AV2-TICKET: Take the "actions_info" into account once it's implemented.
+        # TODO-AV2-TICKET-CREATED: Take the "actions_info" into account once it's implemented.
         return [
             dcc.Download(id={"type": "download_dataframe", "action_id": self._action_id, "target_id": target})
             for target in self.targets

vizro-core/src/vizro/actions/filter_action.py

@@ -17,20 +17,11 @@ def _post_init(self):
         and the calculated arguments.
         """
         # TODO-AV2-OQ: Rethink validation and calculation of properties for filter/parameter/update_figures since they
-        #      have been private actions. Maybe we can make them public and do validation and calculation in _post_init,
-        #      here, instead inside the Filter/Parameter/Page models.
+        #  have been private actions. Maybe we can make them public and do validation and calculation in _post_init,
+        #  here, instead inside the Filter/Parameter/Page models.
 
         self._page_id = model_manager._get_model_page_id(model_id=self._action_id)
 
-        # targets = self._arguments.get("targets")
-        # if targets:
-        #     for target in targets:
-        #         if self._page_id != model_manager._get_model_page_id(model_id=target):
-        #             raise ValueError(f"Component '{target}' does not exist on the page '{self._page_id}'.")
-        # else:
-        #     targets = model_manager._get_page_model_ids_with_figure(page_id=self._page_id)
-        # self._arguments["targets"] = self.targets = targets
-
         # Validate and calculate "targets"
         self.targets = self._arguments.get("targets")
         # Validate and calculate "filter_function"
@@ -75,18 +66,16 @@ def inputs(self):
         from vizro.actions.parameter_action import ParameterAction
         from vizro.actions.filter_interaction_action import FilterInteractionAction
 
-        # TODO-AV2-OO: Consider the following inputs ctx form:
-        #    ```
-        #    return {
-        #        target_1: {'filters': ..., 'parameters': ..., 'filter_interaction': ..., 'theme_selector': ...},
-        #        target_2: {'filters': ..., 'parameters': ..., 'filter_interaction': ..., 'theme_selector': ...},
-        #    }
-        #    ```
-        #    Pros:
-        #    1. We don't need anymore to send all filter/parameters/filter_interaction inputs to the server
-        #    2. Potentially we don't need to dig through the ctx in the _actions_utils.py which decreases the complexity
-        #    Cons:
-        #    1. Time for tests refactoring
+        # TODO-AV2-OQ: Consider the following inputs ctx form:
+        #  ```
+        #  return {
+        #      target_1: {'filters': ..., 'parameters': ..., 'filter_interaction': ..., 'theme_selector': ...},
+        #      target_2: {'filters': ..., 'parameters': ..., 'filter_interaction': ..., 'theme_selector': ...},
+        #  }
+        #  ```
+        #  Pros:
+        #  1. We don't need anymore to send all filter/parameters/filter_interaction inputs to the server
+        #  2. Potentially we don't need to dig through the ctx in the _actions_utils.py which decreases the complexity
 
         page = model_manager[self._page_id]
         return {

vizro-core/src/vizro/actions/parameter_action.py

@@ -17,30 +17,19 @@ def _post_init(self):
         and the calculated arguments.
         """
         # TODO-AV2-OQ: Rethink validation and calculation of properties for filter/parameter/update_figures since they
-        #      have been private actions. Maybe we can make them public and do validation and calculation in _post_init,
-        #      here, instead inside the Filter/Parameter/Page models.
+        #  have been private actions. Maybe we can make them public and do validation and calculation in _post_init,
+        #  here, instead inside the Filter/Parameter/Page models.
 
         # TODO-AV2-OQ: Consider making a difference within this method between 'targets' and 'affected_arguments' e.g.:
-        #   "targets" - only target model IDs e.g. "my_scatter"
-        #   "affected_arguments" - affected_argument per target e.g. "layout.title.size"
-        #   PROS:
-        #   1. Calculate everything we can in advance so we don't have to deal with calculation every time later
+        #  "targets" - only target model IDs e.g. "my_scatter_chart_id"
+        #  "affected_arguments" - affected_argument per target e.g. "layout.title.size"
+        #  PROS:
+        #  1. Calculate everything we can in advance so we don't have to deal with calculation every time later.
 
         self._page_id = model_manager._get_model_page_id(model_id=self._action_id)
 
-        # targets = self._arguments.get("targets")
-        # if targets:
-        #     for target in targets:
-        #         if self._page_id != model_manager._get_model_page_id(model_id=target):
-        #             raise ValueError(f"Component '{target}' does not exist on the page '{self._page_id}'.")
-        # else:
-        #     targets = model_manager._get_page_model_ids_with_figure(page_id=self._page_id)
-        # self._arguments["targets"] = self.targets = targets
-
         # Validate and calculate "targets"
         self.targets = self._arguments.get("targets")
-        # Validate and calculate "filter_function"
-        # Validate and calculate "filter_column"
 
     @staticmethod
     def pure_function(targets: List[str], **inputs: Dict[str, Any]) -> Dict[str, Any]: