From 296a99eff01fbae99607fb4727df4b2d694ee502 Mon Sep 17 00:00:00 2001 From: Maximilian Schulz <83698606+maxschulz-COL@users.noreply.github.com> Date: Sun, 12 May 2024 09:48:46 +0200 Subject: [PATCH] [Bug] Fix bug that prevented custom tables/grids with column referral (#439) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- ...101916_maximilian_schulz_missing_DF_435.md | 47 ++++++++++++ vizro-core/examples/_dev/app.py | 14 +++- .../src/vizro/models/_components/ag_grid.py | 4 +- .../src/vizro/models/_components/table.py | 5 +- .../vizro/models/_components/test_ag_grid.py | 9 +-- .../vizro/models/_components/test_table.py | 9 +-- .../unit/vizro/tables/test_dash_ag_grid.py | 75 ++++++++++++++++--- .../unit/vizro/tables/test_dash_table.py | 72 +++++++++++++++--- 8 files changed, 197 insertions(+), 38 deletions(-) create mode 100644 vizro-core/changelog.d/20240426_101916_maximilian_schulz_missing_DF_435.md diff --git a/vizro-core/changelog.d/20240426_101916_maximilian_schulz_missing_DF_435.md b/vizro-core/changelog.d/20240426_101916_maximilian_schulz_missing_DF_435.md new file mode 100644 index 000000000..6d757f782 --- /dev/null +++ b/vizro-core/changelog.d/20240426_101916_maximilian_schulz_missing_DF_435.md @@ -0,0 +1,47 @@ + + + + + + + + +### Fixed + +- Fix bug that [prevented referring to columns in custom grids/tables](https://github.com/mckinsey/vizro/issues/435) ([#439](https://github.com/mckinsey/vizro/pull/439)) + + diff --git a/vizro-core/examples/_dev/app.py b/vizro-core/examples/_dev/app.py index 592cd1e8f..3bb098360 100644 --- a/vizro-core/examples/_dev/app.py +++ b/vizro-core/examples/_dev/app.py @@ -1,13 +1,19 @@ -"""Rough example used by developers.""" +"""To try out the PR.""" import vizro.models as vm +import vizro.plotly.express as px from vizro import Vizro +df = px.data.iris() + page = vm.Page( - title="Changing the card color", + title="My first dashboard", components=[ - vm.Card(id="my-card", text="""Lorem ipsum dolor sit amet consectetur adipisicing elit."""), - vm.Card(text="""Lorem ipsum dolor sit amet consectetur adipisicing elit."""), + 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"])), ], ) diff --git a/vizro-core/src/vizro/models/_components/ag_grid.py b/vizro-core/src/vizro/models/_components/ag_grid.py index 6bed3e2f0..b3b319433 100644 --- a/vizro-core/src/vizro/models/_components/ag_grid.py +++ b/vizro-core/src/vizro/models/_components/ag_grid.py @@ -110,8 +110,10 @@ def build(self): # The pagination setting (and potentially others) of the initially built AgGrid (in the build method # here) must have the same setting as the object that is built by the on-page-load mechanism using # with the user settings and rendered finally. Otherwise the grid is not rendered correctly. + # Additionally, we cannot remove the DF from the ag grid object before returning it (to save sending + # data over the network), because it breaks filter persistence settings on page change. # Hence be careful when editing the line below. - html.Div(self.__call__(data_frame=pd.DataFrame()), id=self.id, className="table-container"), + html.Div(self.__call__(), id=self.id, className="table-container"), ], color="grey", parent_className="loading-container", diff --git a/vizro-core/src/vizro/models/_components/table.py b/vizro-core/src/vizro/models/_components/table.py index a62067b97..4c681e2a0 100644 --- a/vizro-core/src/vizro/models/_components/table.py +++ b/vizro-core/src/vizro/models/_components/table.py @@ -108,7 +108,10 @@ def build(self): html.Div( [ html.H3(self.title, className="table-title") if self.title else None, - html.Div(self.__call__(data_frame=pd.DataFrame()), id=self.id), + # Please see vm.AgGrid build method as to why we are returning the call with the full data here + # Most of the comments may not apply to the data table, but in order to be consistent, we are + # handling the build method in the exact same way here + html.Div(self.__call__(), id=self.id), ], className="table-container", ), diff --git a/vizro-core/tests/unit/vizro/models/_components/test_ag_grid.py b/vizro-core/tests/unit/vizro/models/_components/test_ag_grid.py index 04eb5c115..d9e1a0cf2 100644 --- a/vizro-core/tests/unit/vizro/models/_components/test_ag_grid.py +++ b/vizro-core/tests/unit/vizro/models/_components/test_ag_grid.py @@ -1,6 +1,5 @@ """Unit tests for vizro.models.AgGrid.""" -import pandas as pd import pytest from asserts import assert_component_equal from dash import dcc, html @@ -117,7 +116,7 @@ def test_pre_build_actions_underlying_ag_grid_id(self, ag_grid_with_id, filter_i class TestBuildAgGrid: - def test_ag_grid_build_mandatory_only(self, standard_ag_grid): + def test_ag_grid_build_mandatory_only(self, standard_ag_grid, gapminder): ag_grid = vm.AgGrid(id="text_ag_grid", figure=standard_ag_grid) ag_grid.pre_build() ag_grid = ag_grid.build() @@ -125,7 +124,7 @@ def test_ag_grid_build_mandatory_only(self, standard_ag_grid): [ None, html.Div( - dash_ag_grid(data_frame=pd.DataFrame(), id="__input_text_ag_grid")(), + dash_ag_grid(data_frame=gapminder, id="__input_text_ag_grid")(), id="text_ag_grid", className="table-container", ), @@ -136,7 +135,7 @@ def test_ag_grid_build_mandatory_only(self, standard_ag_grid): assert_component_equal(ag_grid, expected_ag_grid) - def test_ag_grid_build_with_underlying_id(self, ag_grid_with_id_and_conf, filter_interaction_action): + def test_ag_grid_build_with_underlying_id(self, ag_grid_with_id_and_conf, filter_interaction_action, gapminder): ag_grid = vm.AgGrid(id="text_ag_grid", figure=ag_grid_with_id_and_conf, actions=[filter_interaction_action]) ag_grid.pre_build() ag_grid = ag_grid.build() @@ -146,7 +145,7 @@ def test_ag_grid_build_with_underlying_id(self, ag_grid_with_id_and_conf, filter None, html.Div( dash_ag_grid( - data_frame=pd.DataFrame(), id="underlying_ag_grid_id", dashGridOptions={"pagination": True} + data_frame=gapminder, id="underlying_ag_grid_id", dashGridOptions={"pagination": True} )(), id="text_ag_grid", className="table-container", diff --git a/vizro-core/tests/unit/vizro/models/_components/test_table.py b/vizro-core/tests/unit/vizro/models/_components/test_table.py index cad302344..363eb673c 100644 --- a/vizro-core/tests/unit/vizro/models/_components/test_table.py +++ b/vizro-core/tests/unit/vizro/models/_components/test_table.py @@ -1,6 +1,5 @@ """Unit tests for vizro.models.Table.""" -import pandas as pd import pytest from asserts import assert_component_equal from dash import dcc, html @@ -118,7 +117,7 @@ def test_pre_build_actions_underlying_table_id(self, dash_data_table_with_id, fi class TestBuildTable: - def test_table_build_mandatory_only(self, standard_dash_table): + def test_table_build_mandatory_only(self, standard_dash_table, gapminder): table = vm.Table(id="text_table", figure=standard_dash_table) table.pre_build() table = table.build() @@ -126,7 +125,7 @@ def test_table_build_mandatory_only(self, standard_dash_table): html.Div( [ None, - html.Div(dash_data_table(id="__input_text_table", data_frame=pd.DataFrame())(), id="text_table"), + html.Div(dash_data_table(id="__input_text_table", data_frame=gapminder)(), id="text_table"), ], className="table-container", ), @@ -136,7 +135,7 @@ def test_table_build_mandatory_only(self, standard_dash_table): assert_component_equal(table, expected_table) - def test_table_build_with_underlying_id(self, dash_data_table_with_id, filter_interaction_action): + def test_table_build_with_underlying_id(self, dash_data_table_with_id, filter_interaction_action, gapminder): table = vm.Table(id="text_table", figure=dash_data_table_with_id, actions=[filter_interaction_action]) table.pre_build() table = table.build() @@ -145,7 +144,7 @@ def test_table_build_with_underlying_id(self, dash_data_table_with_id, filter_in html.Div( [ None, - html.Div(dash_data_table(id="underlying_table_id", data_frame=pd.DataFrame())(), id="text_table"), + html.Div(dash_data_table(id="underlying_table_id", data_frame=gapminder)(), id="text_table"), ], className="table-container", ), diff --git a/vizro-core/tests/unit/vizro/tables/test_dash_ag_grid.py b/vizro-core/tests/unit/vizro/tables/test_dash_ag_grid.py index 912d94bdf..c913f3bd8 100644 --- a/vizro-core/tests/unit/vizro/tables/test_dash_ag_grid.py +++ b/vizro-core/tests/unit/vizro/tables/test_dash_ag_grid.py @@ -3,6 +3,7 @@ import vizro.models as vm from asserts import assert_component_equal from dash import dcc, html +from pandas import Timestamp from vizro.models.types import capture from vizro.tables import dash_ag_grid @@ -14,6 +15,26 @@ "date": pd.to_datetime(["2021/01/01", "2021/01/02", "2021/01/03"]), } ) +column_defs = [{"field": "cat"}, {"field": "int"}, {"field": "float"}, {"field": "date"}] +row_data_date_converted = [ + {"cat": "a", "int": 4, "float": 7.3, "date": "2021-01-01"}, + {"cat": "b", "int": 5, "float": 8.2, "date": "2021-01-02"}, + {"cat": "c", "int": 6, "float": 9.1, "date": "2021-01-03"}, +] +row_data_date_raw = [ + {"cat": "a", "date": Timestamp("2021-01-01 00:00:00"), "float": 7.3, "int": 4}, + {"cat": "b", "date": Timestamp("2021-01-02 00:00:00"), "float": 8.2, "int": 5}, + {"cat": "c", "date": Timestamp("2021-01-03 00:00:00"), "float": 9.1, "int": 6}, +] +default_col_defs = { + "filter": True, + "filterParams": {"buttons": ["apply", "reset"], "closeOnApply": True}, + "flex": 1, + "minWidth": 70, + "resizable": True, + "sortable": True, +} +style = {"height": "100%"} class TestDashAgGrid: @@ -22,17 +43,14 @@ def test_dash_ag_grid(self): assert_component_equal( grid, dag.AgGrid( - columnDefs=[{"field": "cat"}, {"field": "int"}, {"field": "float"}, {"field": "date"}], - rowData=[ - {"cat": "a", "int": 4, "float": 7.3, "date": "2021-01-01"}, - {"cat": "b", "int": 5, "float": 8.2, "date": "2021-01-02"}, - {"cat": "c", "int": 6, "float": 9.1, "date": "2021-01-03"}, - ], + columnDefs=column_defs, + rowData=row_data_date_converted, + defaultColDef=default_col_defs, + style=style, ), - keys_to_strip={"className", "defaultColDef", "dashGridOptions", "style"}, + keys_to_strip={"className", "dashGridOptions"}, ) - # we could test other properties such as defaultColDef, - # but this would just test our chosen defaults, and no functionality really + # skipping only dashGridOptions as this is mostly our defaults for data formats, and would crowd the tests class TestCustomDashAgGrid: @@ -58,7 +76,44 @@ def custom_ag_grid(data_frame): [ None, html.Div( - dag.AgGrid(id="__input_custom_ag_grid", columnDefs=[], rowData=[]), + dag.AgGrid(id="__input_custom_ag_grid", columnDefs=column_defs, rowData=row_data_date_raw), + id=id, + className="table-container", + ), + ], + color="grey", + parent_className="loading-container", + ) + + assert_component_equal(custom_grid, expected_grid) + + def test_custom_dash_ag_grid_column_referral(self): + """Tests whether a custom created grid can be correctly built in vm.AgGrid. + + This test focuses on the case that the custom grid includes column referrals on presumed data knowledge. + """ + id = "custom_ag_grid" + + @capture("ag_grid") + def custom_ag_grid(data_frame): + data_frame["cat"] # access "existing" column + return dag.AgGrid( + columnDefs=[{"field": col} for col in data_frame.columns], + rowData=data_frame.to_dict("records"), + ) + + grid_model = vm.AgGrid( + id=id, + figure=custom_ag_grid(data_frame=data), + ) + grid_model.pre_build() + custom_grid = grid_model.build() + + expected_grid = dcc.Loading( + [ + None, + html.Div( + dag.AgGrid(id="__input_custom_ag_grid", columnDefs=column_defs, rowData=row_data_date_raw), id=id, className="table-container", ), diff --git a/vizro-core/tests/unit/vizro/tables/test_dash_table.py b/vizro-core/tests/unit/vizro/tables/test_dash_table.py index 75e4faa47..d8bdf0926 100644 --- a/vizro-core/tests/unit/vizro/tables/test_dash_table.py +++ b/vizro-core/tests/unit/vizro/tables/test_dash_table.py @@ -13,6 +13,17 @@ "date": pd.to_datetime(["2021/01/01", "2021/01/02", "2021/01/03"]), } ) +columns = [ + {"id": "cat", "name": "cat"}, + {"id": "int", "name": "int"}, + {"id": "float", "name": "float"}, + {"id": "date", "name": "date"}, +] +data_in_table = [ + {"cat": "a", "int": 4, "float": 7.3, "date": pd.Timestamp("2021-01-01 00:00:00")}, + {"cat": "b", "int": 5, "float": 8.2, "date": pd.Timestamp("2021-01-02 00:00:00")}, + {"cat": "c", "int": 6, "float": 9.1, "date": pd.Timestamp("2021-01-03 00:00:00")}, +] class TestDashDataTable: @@ -21,17 +32,8 @@ def test_dash_data_table(self): assert_component_equal( table, dash_table.DataTable( - columns=[ - {"id": "cat", "name": "cat"}, - {"id": "int", "name": "int"}, - {"id": "float", "name": "float"}, - {"id": "date", "name": "date"}, - ], - data=[ - {"cat": "a", "int": 4, "float": 7.3, "date": pd.Timestamp("2021-01-01 00:00:00")}, - {"cat": "b", "int": 5, "float": 8.2, "date": pd.Timestamp("2021-01-02 00:00:00")}, - {"cat": "c", "int": 6, "float": 9.1, "date": pd.Timestamp("2021-01-03 00:00:00")}, - ], + columns=columns, + data=data_in_table, style_as_list_view=True, style_data={"border_bottom": "1px solid var(--border-subtle-alpha-01)", "height": "40px"}, style_header={ @@ -70,7 +72,53 @@ def custom_dash_data_table(data_frame): custom_table = table.build() - expected_table_object = custom_dash_data_table(data_frame=pd.DataFrame())() + expected_table_object = dash_table.DataTable( + columns=columns, + data=data_in_table, + ) + expected_table_object.id = "__input_" + id + + expected_table = dcc.Loading( + html.Div( + [ + None, + html.Div(expected_table_object, id=id), + ], + className="table-container", + ), + color="grey", + parent_className="loading-container", + ) + + assert_component_equal(custom_table, expected_table) + + def test_custom_dash_data_table_column_referral(self): + """Tests whether a custom created table callable can be correctly built in vm.Table. + + This test focuses on the case that the custom grid include column referrals on presumed data knowledge. + """ + id = "custom_dash_data_table" + + @capture("table") + def custom_dash_data_table(data_frame): + data_frame["cat"] # access "existing" column + return dash_table.DataTable( + columns=[{"name": col, "id": col} for col in data_frame.columns], + data=data_frame.to_dict("records"), + ) + + table = vm.Table( + id=id, + figure=custom_dash_data_table(data_frame=data), + ) + table.pre_build() + + custom_table = table.build() + + expected_table_object = dash_table.DataTable( + columns=columns, + data=data_in_table, + ) expected_table_object.id = "__input_" + id expected_table = dcc.Loading(