From bf6532953c926b6edbcf714234d2514e84184ca4 Mon Sep 17 00:00:00 2001 From: Li Nguyen <90609403+huong-li-nguyen@users.noreply.github.com> Date: Fri, 19 Jul 2024 11:17:57 +0200 Subject: [PATCH] [Tidy] Improve validation error message if `CapturedCallable` is directly provided (#590) Co-authored-by: Antony Milne --- ...ong_li_nguyen_validation_error_captured.md | 47 +++++++++++++ vizro-core/examples/scratch_dev/app.py | 69 +++++-------------- vizro-core/src/vizro/figures/kpi_cards.py | 12 ++++ .../src/vizro/models/_components/_form.py | 7 +- .../src/vizro/models/_components/container.py | 7 +- .../src/vizro/models/_components/tabs.py | 4 +- vizro-core/src/vizro/models/_models_utils.py | 22 +++++- vizro-core/src/vizro/models/_page.py | 7 +- vizro-core/src/vizro/models/types.py | 15 +++- vizro-core/src/vizro/tables/_dash_ag_grid.py | 10 ++- vizro-core/src/vizro/tables/_dash_table.py | 10 ++- vizro-core/tests/unit/vizro/conftest.py | 11 +++ .../vizro/models/_components/test_figure.py | 32 +++------ .../unit/vizro/models/test_models_utils.py | 27 +++++++- 14 files changed, 193 insertions(+), 87 deletions(-) create mode 100644 vizro-core/changelog.d/20240718_151233_huong_li_nguyen_validation_error_captured.md diff --git a/vizro-core/changelog.d/20240718_151233_huong_li_nguyen_validation_error_captured.md b/vizro-core/changelog.d/20240718_151233_huong_li_nguyen_validation_error_captured.md new file mode 100644 index 000000000..4b6a3accf --- /dev/null +++ b/vizro-core/changelog.d/20240718_151233_huong_li_nguyen_validation_error_captured.md @@ -0,0 +1,47 @@ + + + + + +### Added + +- Added validation error message if `CapturedCallable` is directly provided. ([#590](https://github.com/mckinsey/vizro/pull/590)) + + + + + diff --git a/vizro-core/examples/scratch_dev/app.py b/vizro-core/examples/scratch_dev/app.py index 49775d2be..0a083b512 100644 --- a/vizro-core/examples/scratch_dev/app.py +++ b/vizro-core/examples/scratch_dev/app.py @@ -1,67 +1,34 @@ """Example app to show all features of Vizro.""" -# check out https://github.com/mckinsey/vizro for more info about Vizro -# and checkout https://vizro.readthedocs.io/en/stable/ for documentation - +import pandas as pd import vizro.models as vm import vizro.plotly.express as px from vizro import Vizro +from vizro.figures import kpi_card -df = px.data.iris() - -page = vm.Page( - title="Vizro on PyCafe", - layout=vm.Layout( - grid=[[0, 0, 0, 1, 2, 3], [4, 4, 4, 4, 4, 4], [4, 4, 4, 4, 4, 4], [5, 5, 5, 5, 5, 5], [5, 5, 5, 5, 5, 5]], - row_min_height="175px", - ), - components=[ - vm.Card( - text=""" - ### What is Vizro? +gapminder = px.data.gapminder() - Vizro is a toolkit for creating modular data visualization applications. - """ - ), - vm.Card( - text=""" - ### Github - - Checkout Vizro's github page. - """, - href="https://github.com/mckinsey/vizro", - ), - vm.Card( - text=""" - ### Docs +# data from the demo app +df_kpi = pd.DataFrame( + { + "Actual": [100, 200, 700], + "Reference": [100, 300, 500], + "Category": ["A", "B", "C"], + } +) - Visit the documentation for codes examples, tutorials and API reference. - """, - href="https://vizro.readthedocs.io/", - ), - vm.Card( - text=""" - ### Nav Link - Click this for page 2. - """, - href="/page2", - ), - vm.Graph(id="scatter_chart", figure=px.scatter(df, x="sepal_length", y="petal_width", color="species")), - vm.Graph(id="hist_chart", figure=px.histogram(df, x="sepal_width", color="species")), - ], - controls=[ - vm.Filter(column="species", selector=vm.Dropdown(value=["ALL"])), - vm.Filter(column="petal_length"), - vm.Filter(column="sepal_width"), +home = vm.Page( + title="Page Title", + components=[ + # kpi_card(data_frame=df_kpi, value_column="Actual", title="KPI with value"), + vm.Figure(figure=kpi_card(data_frame=df_kpi, value_column="Actual", title="KPI with value")), ], ) -page2 = vm.Page( - title="Page2", components=[vm.Graph(id="hist_chart2", figure=px.histogram(df, x="sepal_width", color="species"))] -) -dashboard = vm.Dashboard(pages=[page, page2]) +dashboard = vm.Dashboard(pages=[home]) + if __name__ == "__main__": Vizro().build(dashboard).run() diff --git a/vizro-core/src/vizro/figures/kpi_cards.py b/vizro-core/src/vizro/figures/kpi_cards.py index ed60a5305..9d199b2a0 100644 --- a/vizro-core/src/vizro/figures/kpi_cards.py +++ b/vizro-core/src/vizro/figures/kpi_cards.py @@ -56,6 +56,12 @@ def kpi_card( # noqa: PLR0913 Returns: A Dash Bootstrap Components card (`dbc.Card`) containing the formatted KPI value. + Examples: + Wrap inside `vm.Figure` to use as a component inside `vm.Page` or `vm.Container`. + >>> import vizro.models as vm + >>> from vizro.figures import kpi_card + >>> vm.Page(title="Page", components=[vm.Figure(figure=kpi_card(...))]) + """ title = title or f"{agg_func} {value_column}".title() value = data_frame[value_column].agg(agg_func) @@ -119,6 +125,12 @@ def kpi_card_reference( # noqa: PLR0913 Returns: A Dash Bootstrap Components card (`dbc.Card`) containing the formatted KPI value and reference. + Examples: + Wrap inside `vm.Figure` to use as a component inside `vm.Page` or `vm.Container`. + >>> import vizro.models as vm + >>> from vizro.figures import kpi_card_reference + >>> vm.Page(title="Page", components=[vm.Figure(figure=kpi_card_reference(...))]) + """ title = title or f"{agg_func} {value_column}".title() value, reference = data_frame[[value_column, reference_column]].agg(agg_func) diff --git a/vizro-core/src/vizro/models/_components/_form.py b/vizro-core/src/vizro/models/_components/_form.py index 248f66f80..20daf5f51 100644 --- a/vizro-core/src/vizro/models/_components/_form.py +++ b/vizro-core/src/vizro/models/_components/_form.py @@ -12,7 +12,7 @@ from vizro.models import VizroBaseModel from vizro.models._components.form import Checklist, Dropdown, RadioItems, RangeSlider, Slider from vizro.models._layout import set_layout -from vizro.models._models_utils import _log_call, _validate_min_length +from vizro.models._models_utils import _log_call, check_captured_callable, validate_min_length from vizro.models.types import _FormComponentType if TYPE_CHECKING: @@ -34,7 +34,10 @@ class Form(VizroBaseModel): layout: Layout = None # type: ignore[assignment] # Re-used validators - _validate_components = validator("components", allow_reuse=True, always=True)(_validate_min_length) + _check_captured_callable = validator("components", allow_reuse=True, each_item=True, pre=True)( + check_captured_callable + ) + _validate_min_length = validator("components", allow_reuse=True, always=True)(validate_min_length) _validate_layout = validator("layout", allow_reuse=True, always=True)(set_layout) @_log_call diff --git a/vizro-core/src/vizro/models/_components/container.py b/vizro-core/src/vizro/models/_components/container.py index d42d62d8d..899319cc3 100644 --- a/vizro-core/src/vizro/models/_components/container.py +++ b/vizro-core/src/vizro/models/_components/container.py @@ -11,7 +11,7 @@ from vizro.models import VizroBaseModel from vizro.models._layout import set_layout -from vizro.models._models_utils import _log_call, _validate_min_length +from vizro.models._models_utils import _log_call, check_captured_callable, validate_min_length from vizro.models.types import ComponentType if TYPE_CHECKING: @@ -36,7 +36,10 @@ class Container(VizroBaseModel): layout: Layout = None # type: ignore[assignment] # Re-used validators - _validate_components = validator("components", allow_reuse=True, always=True)(_validate_min_length) + _check_captured_callable = validator("components", allow_reuse=True, each_item=True, pre=True)( + check_captured_callable + ) + _validate_min_length = validator("components", allow_reuse=True, always=True)(validate_min_length) _validate_layout = validator("layout", allow_reuse=True, always=True)(set_layout) @_log_call diff --git a/vizro-core/src/vizro/models/_components/tabs.py b/vizro-core/src/vizro/models/_components/tabs.py index e3c3a33f5..763e559cc 100644 --- a/vizro-core/src/vizro/models/_components/tabs.py +++ b/vizro-core/src/vizro/models/_components/tabs.py @@ -11,7 +11,7 @@ from pydantic import validator from vizro.models import VizroBaseModel -from vizro.models._models_utils import _log_call, _validate_min_length +from vizro.models._models_utils import _log_call, validate_min_length if TYPE_CHECKING: from vizro.models._components import Container @@ -29,7 +29,7 @@ class Tabs(VizroBaseModel): type: Literal["tabs"] = "tabs" tabs: List[Container] - _validate_tabs = validator("tabs", allow_reuse=True, always=True)(_validate_min_length) + _validate_tabs = validator("tabs", allow_reuse=True, always=True)(validate_min_length) @_log_call def build(self): diff --git a/vizro-core/src/vizro/models/_models_utils.py b/vizro-core/src/vizro/models/_models_utils.py index 842800d70..58da2bf39 100644 --- a/vizro-core/src/vizro/models/_models_utils.py +++ b/vizro-core/src/vizro/models/_models_utils.py @@ -1,6 +1,8 @@ import logging from functools import wraps +from vizro.models.types import CapturedCallable, _SupportsCapturedCallable + logger = logging.getLogger(__name__) @@ -16,7 +18,21 @@ def _wrapper(self, *args, **kwargs): # Validators for reuse -def _validate_min_length(cls, field): - if not field: +def validate_min_length(cls, value): + if not value: raise ValueError("Ensure this value has at least 1 item.") - return field + return value + + +def check_captured_callable(cls, value): + if isinstance(value, CapturedCallable): + captured_callable = value + elif isinstance(value, _SupportsCapturedCallable): + captured_callable = value._captured_callable + else: + return value + + raise ValueError( + f"A callable of mode `{captured_callable._mode}` has been provided. Please wrap it inside " + f"`{captured_callable._model_example}`." + ) diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 558d672c7..cf0fa125f 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -16,7 +16,7 @@ from vizro.models import Action, Layout, VizroBaseModel from vizro.models._action._actions_chain import ActionsChain, Trigger from vizro.models._layout import set_layout -from vizro.models._models_utils import _log_call, _validate_min_length +from vizro.models._models_utils import _log_call, check_captured_callable, validate_min_length from .types import ComponentType, ControlType @@ -52,7 +52,10 @@ class Page(VizroBaseModel): actions: List[ActionsChain] = [] # Re-used validators - _validate_components = validator("components", allow_reuse=True, always=True)(_validate_min_length) + _check_captured_callable = validator("components", allow_reuse=True, each_item=True, pre=True)( + check_captured_callable + ) + _validate_min_length = validator("components", allow_reuse=True, always=True)(validate_min_length) _validate_layout = validator("layout", allow_reuse=True, always=True)(set_layout) @root_validator(pre=True) diff --git a/vizro-core/src/vizro/models/types.py b/vizro-core/src/vizro/models/types.py index dba2eceb2..8dce74d74 100644 --- a/vizro-core/src/vizro/models/types.py +++ b/vizro-core/src/vizro/models/types.py @@ -96,8 +96,9 @@ def __init__(self, function, /, *args, **kwargs): self.__bound_arguments.update(self.__bound_arguments[var_keyword_param]) del self.__bound_arguments[var_keyword_param] - # This is used to check that the mode of the capture decorator matches the inserted captured callable. + # Used in later validations of the captured callable. self._mode = None + self._model_example = None def __call__(self, *args, **kwargs): """Run the `function` with the initially bound arguments overridden by `**kwargs`. @@ -283,7 +284,16 @@ class capture: def __init__(self, mode: Literal["graph", "action", "table", "ag_grid", "figure"]): """Decorator to capture a function call.""" + # mode and model_example are used in later validations of the captured callable. self._mode = mode + model_examples = { + "graph": "vm.Graph(figure=...)", + "action": "vm.Action(function=...)", + "table": "vm.Table(figure=...)", + "ag_grid": "vm.AgGrid(figure=...)", + "figure": "vm.Figure(figure=...)", + } + self._model_example = model_examples[mode] def __call__(self, func, /): """Produces a CapturedCallable or _DashboardReadyFigure. @@ -307,6 +317,7 @@ def wrapped(*args, **kwargs) -> _DashboardReadyFigure: # positional or keyword, this is much more robust than trying to get it out of arg or kwargs ourselves. captured_callable: CapturedCallable = CapturedCallable(func, *args, **kwargs) captured_callable._mode = self._mode + captured_callable._model_example = self._model_example try: captured_callable["data_frame"] @@ -334,6 +345,7 @@ def wrapped(*args, **kwargs): # Note this is basically the same as partial(func, *args, **kwargs) captured_callable: CapturedCallable = CapturedCallable(func, *args, **kwargs) captured_callable._mode = self._mode + captured_callable._model_example = self._model_example return captured_callable return wrapped @@ -346,6 +358,7 @@ def wrapped(*args, **kwargs): captured_callable: CapturedCallable = CapturedCallable(func, *args, **kwargs) captured_callable._mode = self._mode + captured_callable._model_example = self._model_example try: captured_callable["data_frame"] diff --git a/vizro-core/src/vizro/tables/_dash_ag_grid.py b/vizro-core/src/vizro/tables/_dash_ag_grid.py index cee45f99c..dec4efeca 100644 --- a/vizro-core/src/vizro/tables/_dash_ag_grid.py +++ b/vizro-core/src/vizro/tables/_dash_ag_grid.py @@ -48,7 +48,15 @@ @capture("ag_grid") def dash_ag_grid(data_frame: pd.DataFrame, **kwargs) -> dag.AgGrid: - """Implementation of `dash_ag_grid.AgGrid` with sensible defaults to be used in [`AgGrid`][vizro.models.AgGrid].""" + """Implementation of `dash_ag_grid.AgGrid` with sensible defaults to be used in [`AgGrid`][vizro.models.AgGrid]. + + Examples + Wrap inside `vm.AgGrid` to use as a component inside `vm.Page` or `vm.Container`. + >>> import vizro.models as vm + >>> from vizro.tables import dash_ag_grid + >>> vm.Page(title="Page", components=[vm.AgGrid(figure=dash_ag_grid(...))]) + + """ defaults = { "className": "ag-theme-quartz-dark ag-theme-vizro", "columnDefs": [{"field": col} for col in data_frame.columns], diff --git a/vizro-core/src/vizro/tables/_dash_table.py b/vizro-core/src/vizro/tables/_dash_table.py index da6087b78..d55d818c3 100644 --- a/vizro-core/src/vizro/tables/_dash_table.py +++ b/vizro-core/src/vizro/tables/_dash_table.py @@ -9,7 +9,15 @@ @capture("table") def dash_data_table(data_frame: pd.DataFrame, **kwargs) -> dash_table.DataTable: - """Standard `dash_table.DataTable` with sensible defaults to be used in [`Table`][vizro.models.Table].""" + """Standard `dash_table.DataTable` with sensible defaults to be used in [`Table`][vizro.models.Table]. + + Examples + Wrap inside `vm.Table` to use as a component inside `vm.Page` or `vm.Container`. + >>> import vizro.models as vm + >>> from vizro.table import dash_data_table + >>> vm.Page(title="Page", components=[vm.Table(figure=dash_data_table(...))]) + + """ defaults = { "columns": [{"name": col, "id": col} for col in data_frame.columns], "style_as_list_view": True, diff --git a/vizro-core/tests/unit/vizro/conftest.py b/vizro-core/tests/unit/vizro/conftest.py index e7ab6168f..67136ead9 100644 --- a/vizro-core/tests/unit/vizro/conftest.py +++ b/vizro-core/tests/unit/vizro/conftest.py @@ -5,6 +5,7 @@ import vizro.models as vm import vizro.plotly.express as px from vizro import Vizro +from vizro.figures import kpi_card from vizro.tables import dash_ag_grid, dash_data_table @@ -61,6 +62,16 @@ def standard_go_chart(gapminder): return go.Figure(data=go.Scatter(x=gapminder["gdpPercap"], y=gapminder["lifeExp"], mode="markers")) +@pytest.fixture +def standard_kpi_card(gapminder): + return kpi_card( + data_frame=gapminder, + value_column="lifeExp", + agg_func="mean", + value_format="{value:.3f}", + ) + + @pytest.fixture def chart_with_temporal_data(stocks): return go.Figure(data=go.Scatter(x=stocks["Date"], y=stocks["AAPL.High"], mode="markers")) diff --git a/vizro-core/tests/unit/vizro/models/_components/test_figure.py b/vizro-core/tests/unit/vizro/models/_components/test_figure.py index 45ebdf215..1c1a5e9d5 100644 --- a/vizro-core/tests/unit/vizro/models/_components/test_figure.py +++ b/vizro-core/tests/unit/vizro/models/_components/test_figure.py @@ -16,16 +16,6 @@ from vizro.managers import data_manager -@pytest.fixture -def kpi_card_with_dataframe(gapminder): - return kpi_card( - data_frame=gapminder, - value_column="lifeExp", - agg_func="mean", - value_format="{value:.3f}", - ) - - @pytest.fixture def kpi_card_with_str_dataframe(): return kpi_card( @@ -37,12 +27,12 @@ def kpi_card_with_str_dataframe(): class TestFigureInstantiation: - def test_create_figure_mandatory_only(self, kpi_card_with_dataframe): - figure = vm.Figure(figure=kpi_card_with_dataframe) + def test_create_figure_mandatory_only(self, standard_kpi_card): + figure = vm.Figure(figure=standard_kpi_card) assert hasattr(figure, "id") assert figure.type == "figure" - assert figure.figure == kpi_card_with_dataframe + assert figure.figure == standard_kpi_card def test_captured_callable_invalid(self, standard_go_chart): with pytest.raises( @@ -66,15 +56,15 @@ def test_captured_callable_wrong_mode(self, standard_dash_table): class TestDunderMethodsFigure: - def test_getitem_known_args(self, kpi_card_with_dataframe): - figure = vm.Figure(figure=kpi_card_with_dataframe) + def test_getitem_known_args(self, standard_kpi_card): + figure = vm.Figure(figure=standard_kpi_card) assert figure["value_column"] == "lifeExp" assert figure["agg_func"] == "mean" assert figure["value_format"] == "{value:.3f}" assert figure["type"] == "figure" - def test_getitem_unknown_args(self, kpi_card_with_dataframe): - figure = vm.Figure(figure=kpi_card_with_dataframe) + def test_getitem_unknown_args(self, standard_kpi_card): + figure = vm.Figure(figure=standard_kpi_card) with pytest.raises(KeyError): figure["unknown_args"] @@ -85,14 +75,14 @@ def test_process_figure_data_frame_str_df(self, kpi_card_with_str_dataframe, gap figure = vm.Figure(figure=kpi_card_with_str_dataframe) assert data_manager[figure["data_frame"]].load().equals(gapminder) - def test_process_figure_data_frame_df(self, kpi_card_with_dataframe, gapminder): - figure = vm.Figure(figure=kpi_card_with_dataframe) + def test_process_figure_data_frame_df(self, standard_kpi_card, gapminder): + figure = vm.Figure(figure=standard_kpi_card) assert data_manager[figure["data_frame"]].load().equals(gapminder) class TestFigureBuild: - def test_figure_build(self, kpi_card_with_dataframe, gapminder): - figure = vm.Figure(id="figure-id", figure=kpi_card_with_dataframe).build() + def test_figure_build(self, standard_kpi_card, gapminder): + figure = vm.Figure(id="figure-id", figure=standard_kpi_card).build() expected_figure = dcc.Loading( html.Div( diff --git a/vizro-core/tests/unit/vizro/models/test_models_utils.py b/vizro-core/tests/unit/vizro/models/test_models_utils.py index c3a1a312e..24e6808be 100644 --- a/vizro-core/tests/unit/vizro/models/test_models_utils.py +++ b/vizro-core/tests/unit/vizro/models/test_models_utils.py @@ -11,10 +11,35 @@ class TestSharedValidators: - def test_set_components_validator(self, model_with_layout): + def test_validate_min_length(self, model_with_layout): with pytest.raises(ValidationError, match="Ensure this value has at least 1 item."): model_with_layout(title="Title", components=[]) + @pytest.mark.parametrize( + "captured_callable, error_message", + [ + ( + "standard_px_chart", + "A callable of mode `graph` has been provided. Please wrap it inside `vm.Graph(figure=...)`", + ), + ( + "standard_ag_grid", + "A callable of mode `ag_grid` has been provided. Please wrap it inside `vm.AgGrid(figure=...)`", + ), + ( + "standard_dash_table", + "A callable of mode `table` has been provided. Please wrap it inside `vm.Table(figure=...)`", + ), + ( + "standard_kpi_card", + "A callable of mode `figure` has been provided. Please wrap it inside `vm.Figure(figure=...)`", + ), + ], + ) + def test_check_captured_callable(self, model_with_layout, captured_callable, error_message, request): + with pytest.raises(ValidationError, match=re.escape(error_message)): + model_with_layout(title="Title", components=[request.getfixturevalue(captured_callable)]) + def test_check_for_valid_component_types(self, model_with_layout): with pytest.raises( ValidationError,