[Bug] Fix inconsistent return types in selector values

vizro-core/changelog.d/20240703_091955_huong_li_nguyen_fix_selector_return_type.md

@@ -0,0 +1,47 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Highlights ✨
+
+- A bullet item for the Highlights ✨ category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
+
+-->
+<!--
+### Removed
+
+- A bullet item for the Removed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
+
+-->
+<!--
+### Added
+
+- A bullet item for the Added category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
+
+-->
+
+### Fixed
+
+- Ensure that categorical selectors always return a list of values. ([#562](https://github.com/mckinsey/vizro/pull/562))
+
+<!--
+### Security
+
+- A bullet item for the Security category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
+
+-->

vizro-core/examples/_dev/app.py

@@ -1,64 +1,49 @@
 """Dev app to try things out."""
 
-from typing import Optional
-
-import dash_bootstrap_components as dbc
 import pandas as pd
 import vizro.models as vm
 import vizro.plotly.express as px
-from dash import html
 from vizro import Vizro
-from vizro.figures import kpi_card
 from vizro.models.types import capture
 
-tips = px.data.tips
+df_stocks = px.data.stocks(datetimes=True)
+df_stocks_long = pd.melt(
+    df_stocks,
+    id_vars="date",
+    value_vars=["GOOG", "AAPL", "AMZN", "FB", "NFLX", "MSFT"],
+    var_name="stocks",
+    value_name="value",
+)
 
 
-@capture("figure")  # (1)!
-def custom_kpi_card(  # noqa: PLR0913
-    data_frame: pd.DataFrame,
-    value_column: str,
-    *,
-    value_format: str = "{value}",
-    agg_func: str = "sum",
-    title: Optional[str] = None,
-    icon: Optional[str] = None,
-) -> dbc.Card:  # (2)!
-    """Creates a custom KPI card."""
-    title = title or f"{agg_func} {value_column}".title()
-    value = data_frame[value_column].agg(agg_func)
+@capture("graph")
+def vizro_plot(data_frame, stocks_selected, **kwargs):
+    """Custom chart function."""
+    return px.line(data_frame[data_frame["stocks"].isin(stocks_selected)], **kwargs)
 
-    header = dbc.CardHeader(
-        [
-            html.H2(title),
-            html.P(icon, className="material-symbols-outlined") if icon else None,  # (3)!
-        ]
-    )
-    body = dbc.CardBody([value_format.format(value=value)])
-    return dbc.Card([header, body], className="card-kpi")
 
+df_stocks_long["value"] = df_stocks_long["value"].round(3)
 
 page = vm.Page(
-    title="Create your own KPI card",
-    layout=vm.Layout(grid=[[0, 1, -1, -1]] + [[-1, -1, -1, -1]] * 3),  # (4)!
+    title="My first page",
     components=[
-        vm.Figure(
-            figure=kpi_card(  # (5)!
-                data_frame=tips,
-                value_column="tip",
-                value_format="${value:.2f}",
-                icon="shopping_cart",
-                title="Default KPI card",
-            )
+        vm.Graph(
+            id="my_graph",
+            figure=vizro_plot(
+                data_frame=df_stocks_long,
+                stocks_selected=list(df_stocks_long["stocks"].unique()),
+                x="date",
+                y="value",
+                color="stocks",
+            ),
         ),
-        vm.Figure(
-            figure=custom_kpi_card(  # (6)!
-                data_frame=tips,
-                value_column="tip",
-                value_format="${value:.2f}",
-                icon="payment",
-                title="Custom KPI card",
-            )
+    ],
+    controls=[
+        vm.Parameter(
+            targets=["my_graph.stocks_selected"],
+            selector=vm.Dropdown(
+                options=[{"label": s, "value": s} for s in df_stocks_long["stocks"].unique()],
+            ),
         ),
     ],
 )

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

@@ -138,12 +138,19 @@ def _get_parametrized_config(target: ModelID, ctd_parameters: List[CallbackTrigg
     config["data_frame"] = {}
 
     for ctd in ctd_parameters:
-        selector_value = ctd[
-            "value"
-        ]  # TODO: needs to be refactored so that it is independent of implementation details
+        # TODO: needs to be refactored so that it is independent of implementation details
+        selector_value = ctd["value"]
+
         if hasattr(selector_value, "__iter__") and ALL_OPTION in selector_value:  # type: ignore[operator]
             selector: SelectorType = model_manager[ctd["id"]]
-            selector_value = selector.options
+
+            # Even if options are provided as List[Dict], the Dash component only returns a List of values.
+            # So we need to ensure that we always return a List only as well to provide consistent types.
+            if all(isinstance(option, dict) for option in selector.options):
+                selector_value = [option["value"] for option in selector.options]
+            else:
+                selector_value = selector.options
+
         selector_value = _validate_selector_value_none(selector_value)
         selector_actions = _get_component_actions(model_manager[ctd["id"]])
 

vizro-core/src/vizro/models/_action/_action.py

@@ -105,7 +105,7 @@ def _action_callback_function(
     ) -> Any:
         logger.debug("===== Running action with id %s, function %s =====", self.id, self.function._function.__name__)
         if logger.isEnabledFor(logging.DEBUG):
-            logger.debug("Action inputs:\n%s", pformat(inputs, depth=2, width=200))
+            logger.debug("Action inputs:\n%s", pformat(inputs, depth=3, width=200))
             logger.debug("Action outputs:\n%s", pformat(outputs, width=200))
 
         if isinstance(inputs, Mapping):

vizro-core/tests/unit/vizro/actions/conftest.py

@@ -10,6 +10,11 @@ def gapminder_2007(gapminder):
     return gapminder.query("year == 2007")
 
 
+@pytest.fixture
+def iris():
+    return px.data.iris()
+
+
 @pytest.fixture
 def gapminder_dynamic_first_n_last_n_function(gapminder):
     return lambda first_n=None, last_n=None: (
@@ -44,6 +49,16 @@ def scatter_chart(gapminder_2007, scatter_params):
     return px.scatter(gapminder_2007, **scatter_params).update_layout(margin_t=24)
 
 
+@pytest.fixture
+def scatter_matrix_params():
+    return {"dimensions": ["sepal_width", "sepal_length", "petal_width", "petal_length"]}
+
+
+@pytest.fixture
+def scatter_matrix_chart(iris, scatter_matrix_params):
+    return px.scatter_matrix(iris, **scatter_matrix_params).update_layout(margin_t=24)
+
+
 @pytest.fixture
 def scatter_chart_dynamic_data_frame(scatter_params):
     return px.scatter("gapminder_dynamic_first_n_last_n", **scatter_params).update_layout(margin_t=24)
@@ -110,3 +125,16 @@ def managers_one_page_two_graphs_one_table_one_aggrid_one_button(
         ],
     )
     Vizro._pre_build()
+
+
+@pytest.fixture
+def managers_one_page_one_graph_with_dict_param_input(scatter_matrix_chart):
+    """Instantiates a model_manager and data_manager with a page and a graph that requires a list input."""
+    vm.Page(
+        id="test_page",
+        title="My first dashboard",
+        components=[
+            vm.Graph(id="scatter_matrix_chart", figure=scatter_matrix_chart),
+        ],
+    )
+    Vizro._pre_build()

vizro-core/tests/unit/vizro/actions/test_parameter_action.py

@@ -15,6 +15,13 @@ def target_scatter_parameter_y(request, gapminder_2007, scatter_params):
     return px.scatter(gapminder_2007, **scatter_params).update_layout(margin_t=24)
 
 
+@pytest.fixture
+def target_scatter_matrix_parameter_dimensions(request, iris, scatter_matrix_params):
+    dimensions = request.param
+    scatter_matrix_params["dimensions"] = dimensions
+    return px.scatter_matrix(iris, **scatter_matrix_params).update_layout(margin_t=24)
+
+
 @pytest.fixture
 def target_scatter_parameter_hover_data(request, gapminder_2007, scatter_params):
     hover_data = request.param
@@ -95,6 +102,38 @@ def ctx_parameter_y(request):
     return context_value
 
 
+@pytest.fixture
+def ctx_parameter_dimensions(request):
+    """Mock dash.ctx that represents `dimensions` Parameter value selection."""
+    y = request.param
+    mock_ctx = {
+        "args_grouping": {
+            "external": {
+                "filter_interaction": [],
+                "filters": [],
+                "parameters": [
+                    CallbackTriggerDict(
+                        id="dimensions_parameter",
+                        property="value",
+                        value=y,
+                        str_id="dimensions_parameter",
+                        triggered=False,
+                    )
+                ],
+                "theme_selector": CallbackTriggerDict(
+                    id="theme_selector",
+                    property="checked",
+                    value=False,
+                    str_id="theme_selector",
+                    triggered=False,
+                ),
+            }
+        }
+    }
+    context_value.set(AttributeDict(**mock_ctx))
+    return context_value
+
+
 @pytest.fixture
 def ctx_parameter_hover_data(request):
     """Mock dash.ctx that represents hover_data Parameter value selection."""
@@ -497,3 +536,36 @@ def test_data_frame_parameters_multiple_targets(
         }
 
         assert result == expected
+
+    @pytest.mark.usefixtures("managers_one_page_one_graph_with_dict_param_input")
+    @pytest.mark.parametrize(
+        "ctx_parameter_dimensions, target_scatter_matrix_parameter_dimensions",
+        [("ALL", ["sepal_length", "sepal_width", "petal_length", "petal_width"]), (["sepal_width"], ["sepal_width"])],
+        indirect=True,
+    )
+    def test_one_parameter_with_dict_input_as_options(
+        self, ctx_parameter_dimensions, target_scatter_matrix_parameter_dimensions
+    ):
+        # If the options are provided as a list of dictionaries, the value should be correctly passed to the
+        # target as a list. So when "ALL" is selected, a list of all possible values should be returned.
+        dimensions_parameter = vm.Parameter(
+            id="test_parameter_dimensions",
+            targets=["scatter_matrix_chart.dimensions"],
+            selector=vm.RadioItems(
+                id="dimensions_parameter",
+                options=[
+                    {"label": "sepal_length", "value": "sepal_length"},
+                    {"label": "sepal_width", "value": "sepal_width"},
+                    {"label": "petal_length", "value": "petal_length"},
+                    {"label": "petal_width", "value": "petal_width"},
+                ],
+            ),
+        )
+        model_manager["test_page"].controls = [dimensions_parameter]
+        dimensions_parameter.pre_build()
+
+        # Run action by picking the above added action function and executing it with ()
+        result = model_manager[f"{PARAMETER_ACTION_PREFIX}_test_parameter_dimensions"].function()
+        expected = {"scatter_matrix_chart": target_scatter_matrix_parameter_dimensions}
+
+        assert result == expected