Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tidy] Return empty data frame from AgGrid, Table and Figure build methods #644

Merged
merged 22 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9df7be6
How to reproduce a ag_grid bug
petar-qb Aug 8, 2024
b9a0289
AgGrid tested
petar-qb Aug 20, 2024
9406791
First version
petar-qb Aug 20, 2024
a711948
Deleting unnecessary my_dev example
petar-qb Aug 20, 2024
83000ea
Returned empty data_frame from figure component build method
petar-qb Aug 21, 2024
77871f8
Conflicts solved
petar-qb Aug 21, 2024
f2c21f9
changelog file added
petar-qb Aug 21, 2024
cb8e8e5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 21, 2024
68ae21c
Revert a minor change
petar-qb Aug 21, 2024
d21e46b
Reverting Figure build comment about self.id location
petar-qb Aug 22, 2024
90c54da
changelog typo
petar-qb Aug 22, 2024
7ef8b6d
Minor changes
petar-qb Aug 22, 2024
8368e6d
Merge branch 'main' of https://github.com/mckinsey/vizro into feat/re…
petar-qb Aug 22, 2024
5d650ae
Merge branch 'main' of https://github.com/mckinsey/vizro into feat/re…
petar-qb Aug 22, 2024
a42ec2d
Merge branch 'main' of https://github.com/mckinsey/vizro into feat/re…
petar-qb Aug 23, 2024
006ba35
Additional optimization for AgGrid
maxschulz-COL Aug 23, 2024
44fd247
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 23, 2024
2596a87
Addressing PR comments
petar-qb Aug 26, 2024
8877bb9
Merge branch 'main' of https://github.com/mckinsey/vizro into feat/re…
petar-qb Aug 26, 2024
639e175
Merge branch 'main' of https://github.com/mckinsey/vizro into feat/re…
petar-qb Aug 26, 2024
ce00213
Reverting dash_ag_grid columnDefs optimisation
petar-qb Aug 26, 2024
30f5917
Merge branch 'main' of https://github.com/mckinsey/vizro into feat/re…
petar-qb Aug 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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

- Return an empty data on initial build of AgGrid, Table and Figure objects. ([#644](https://github.com/mckinsey/vizro/pull/644))
- Reduce number of figure function calls for AgGrid, Table and Figure objects. ([#644](https://github.com/mckinsey/vizro/pull/644))
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
petar-qb marked this conversation as resolved.
Show resolved Hide resolved

<!--
### 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

- Fix persistence of `columnSize` and `selectedRows` for AgGrid. ([#644](https://github.com/mckinsey/vizro/pull/644))
petar-qb marked this conversation as resolved.
Show resolved Hide resolved

<!--
### 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))

-->
163 changes: 117 additions & 46 deletions vizro-core/examples/scratch_dev/app.py
Original file line number Diff line number Diff line change
@@ -1,67 +1,138 @@
"""Dev app to try things out."""

import pandas as pd
import vizro.models as vm
import vizro.plotly.express as px
from charts.charts import page2
from vizro import Vizro
from vizro.actions import filter_interaction
from vizro.figures import kpi_card
from vizro.managers import data_manager
from vizro.models.types import capture
from vizro.tables import dash_ag_grid, dash_data_table

data = pd.DataFrame(
{
"Column 1": [1, 2, 3, 4, 5, 6],
"Column 2": ["A", "B", "C", "VeryLongStringInputCell_VeryLongStringInputCell", "D", "E"],
"Column 3": [10.5, 20.1, 30.2, 40.3, 50.4, 60.5],
}
)

iris_dataset = px.data.iris()
giant_dataset = pd.concat([iris_dataset] * 10000, ignore_index=True)

def load_data():
"""Load data."""
return data


data_manager["my_data"] = load_data


@capture("ag_grid")
def custom_dash_ag_grid(data_frame, **kwargs):
"""Custom AgGrid."""
# print(f"dash_ag_grid -> len: {len(data_frame)}")
return dash_ag_grid(data_frame, **kwargs)()


@capture("table")
def custom_dash_data_table(data_frame, **kwargs):
"""Custom DataTable."""
# print(f"dash_data_table -> len: {len(data_frame)}")
return dash_data_table(data_frame, **kwargs)()


@capture("graph")
def custom_px_scatter(data_frame, **kwargs):
"""Custom Scatter plot."""
# print(f"graph -> len: {len(data_frame)}")
return px.scatter(data_frame, **kwargs)

df = px.data.iris()

data_manager["iris"] = px.data.iris()
@capture("figure")
def custom_kpi_card(data_frame, **kwargs):
"""Custom KPI card."""
# print(f"kpi_card -> len: {len(data_frame)}")
return kpi_card(data_frame, **kwargs)()

page = vm.Page(
title="Test",

page_grid = vm.Page(
title="Example Page",
layout=vm.Layout(
grid=[[0, 1], [2, 3], [4, 5]],
grid=[
[0, 1],
[2, 3],
]
),
components=[
vm.Card(
text="""
### What is Vizro?

Vizro is a toolkit for creating modular data visualization applications.
"""
vm.AgGrid(
id="outer_ag_grid_id",
figure=custom_dash_ag_grid(
id="inner_ag_grid_id",
data_frame="my_data",
columnDefs=[{"field": col, "checkboxSelection": True, "filter": True} for col in data.columns],
columnSize="autoSize",
defaultColDef={"resizable": True},
persistence=True,
persistence_type="local",
persisted_props=["selectedRows", "filterModel"],
dashGridOptions={
"rowSelection": "multiple",
"suppressRowClickSelection": True,
# Turn pagination on to see results:
# "pagination": True,
# "paginationPageSize": 3,
},
),
actions=[
vm.Action(
function=filter_interaction(targets=["graph_id"]),
)
],
),
vm.Card(
text="""
### Github

Checkout Vizro's github page.
""",
href="https://github.com/mckinsey/vizro",
vm.Table(
id="outer_table_id",
figure=custom_dash_data_table(
id="inner_table_id",
data_frame="my_data",
row_selectable="multi",
filter_action="native",
persistence=True,
persistence_type="local",
# Turn pagination on to see results:
# page_action="native",
# page_size=3,
),
),
vm.Card(
text="""
### Docs

Visit the documentation for codes examples, tutorials and API reference.
""",
href="https://vizro.readthedocs.io/",
vm.Graph(
id="graph_id",
figure=custom_px_scatter(data_frame="my_data", x="Column 1", y="Column 3"),
),
vm.Card(
text="""
### Nav Link

Click this for page 2.
""",
href="/page2",
vm.Figure(
id="figure_id",
figure=custom_kpi_card(data_frame="my_data", value_column="Column 1"),
),
vm.Graph(id="scatter_chart", figure=px.scatter("iris", x="sepal_length", y="petal_width", color="species")),
vm.Graph(id="hist_chart", figure=px.histogram("iris", 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"),
],
controls=[vm.Filter(column="Column 1", selector=vm.RangeSlider(step=1))],
)

dashboard = vm.Dashboard(pages=[page, page2])
columnDefs = [{"field": "petal_length"}]

if __name__ == "__main__":
from vizro import Vizro
dashboard = vm.Dashboard(
pages=[
vm.Page(
title="Page_1",
components=[vm.Card(text="Dummy page just for testing")],
),
page_grid,
vm.Page(
title="Page_3",
components=[vm.AgGrid(figure=dash_ag_grid(data_frame=giant_dataset,columnDefs=columnDefs))],
),
]
)

string = dashboard._to_python(extra_imports={"from dash_ag_grid import AgGrid"})
print(string) # noqa

Vizro().build(dashboard).run()
if __name__ == "__main__":
Vizro().build(dashboard).run(debug=True)
8 changes: 7 additions & 1 deletion vizro-core/src/vizro/_vizro.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,13 @@ def __init__(self, **kwargs):
[Dash documentation](https://dash.plotly.com/reference#dash.dash) for possible arguments.

"""
self.dash = dash.Dash(**kwargs, use_pages=True, pages_folder="", title="Vizro")
self.dash = dash.Dash(
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
**kwargs,
pages_folder="",
suppress_callback_exceptions=True,
title="Vizro",
use_pages=True,
)
petar-qb marked this conversation as resolved.
Show resolved Hide resolved

# Include Vizro assets (in the static folder) as external scripts and stylesheets. We extend self.dash.config
# objects so the user can specify additional external_scripts and external_stylesheets via kwargs.
Expand Down
5 changes: 5 additions & 0 deletions vizro-core/src/vizro/actions/_actions_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ def _get_modified_page_figures(
) -> Dict[str, Any]:
targets = targets or []

# This only exists to simulate a delay in the on_page_load or filter actions.
# from time import sleep

# sleep(1.5)

petar-qb marked this conversation as resolved.
Show resolved Hide resolved
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
filtered_data, parameterized_config = _get_targets_data_and_config(
ctds_filter=ctds_filter,
ctds_filter_interaction=ctds_filter_interaction,
Expand Down
19 changes: 12 additions & 7 deletions vizro-core/src/vizro/models/_components/ag_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,18 @@ def build(self):
return dcc.Loading(
children=[
html.H3(self.title) if self.title else None,
# 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__(), id=self.id, className="table-container"),
# The Div component with `id=self._input_component_id` is rendered during the build phase.
# This placeholder component is quickly replaced by the actual AgGrid object, which is generated using
# a filtered data_frame and parameterized arguments as part of the on_page_load mechanism.
# To prevent pagination and persistence issues while maintaining a lightweight component initial load,
# this method now returns a html.Div object instead of the previous dag.AgGrid. The actual AgGrid is
# then rendered by the on_page_load mechanism.
# The `id=self._input_component_id` is set to avoid the "Non-existing object" Dash exception.
html.Div(
id=self.id,
children=[html.Div(id=self._input_component_id)],
className="table-container",
),
],
color="grey",
parent_className="loading-container",
Expand Down
9 changes: 6 additions & 3 deletions vizro-core/src/vizro/models/_components/figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,14 @@ def __getitem__(self, arg_name: str):
@_log_call
def build(self):
return dcc.Loading(
# Refer to the vm.AgGrid build method for details on why we return the
# html.Div(id=self.id) instead of actual figure object with the original data_frame.
# Optimally, we would like to provide id=self.id directly here such that we can target the CSS
# of the children via ID as well, but the `id` doesn't seem to be passed on to the loading component.
# I've raised an issue on dash here: https://github.com/plotly/dash/issues/2878
# of the children via ID as well, but the `id` doesn't seem to be passed on to the loading component.
# This limitation is handled with this PR -> https://github.com/plotly/dash/pull/2888.
# The PR is merged but is not released yet. Once it is released, we can try to refactor the following code.
# In the meantime, we are adding an extra html.div here.
html.Div(self.__call__(), id=self.id, className="figure-container"),
html.Div(id=self.id, className="figure-container"),
color="grey",
parent_className="loading-container",
overlay_style={"visibility": "visible", "opacity": 0.3},
Expand Down
11 changes: 7 additions & 4 deletions vizro-core/src/vizro/models/_components/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,13 @@ def build(self):
return dcc.Loading(
children=[
html.H3(self.title) if self.title else None,
# 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"),
# Refer to the vm.AgGrid build method for details on why we return the
# html.Div(id=self._input_component_id) instead of actual figure object with the original data_frame.
html.Div(
id=self.id,
children=[html.Div(id=self._input_component_id)],
className="table-container",
),
],
color="grey",
parent_className="loading-container",
Expand Down
3 changes: 3 additions & 0 deletions vizro-core/src/vizro/tables/_dash_ag_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ def dash_ag_grid(data_frame: pd.DataFrame, **kwargs) -> dag.AgGrid:
>>> vm.Page(title="Page", components=[vm.AgGrid(figure=dash_ag_grid(...))])

"""
if "columnDefs" in kwargs:
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
used_columns = [col["field"] for col in kwargs["columnDefs"] if col["field"] in data_frame.columns]
data_frame = data_frame[used_columns]
defaults = {
"className": "ag-theme-quartz-dark ag-theme-vizro",
"columnDefs": [{"field": col} for col in data_frame.columns],
Expand Down
21 changes: 19 additions & 2 deletions vizro-core/tests/unit/vizro/models/_components/test_ag_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
from vizro.tables import dash_ag_grid


@pytest.fixture
def dash_ag_grid_with_id():
return dash_ag_grid(id="underlying_table_id", data_frame=px.data.gapminder())


@pytest.fixture
def dash_ag_grid_with_arguments():
return dash_ag_grid(data_frame=px.data.gapminder(), defaultColDef={"resizable": False, "sortable": False})
Expand Down Expand Up @@ -97,6 +102,18 @@ def test_getitem_unknown_args(self, standard_ag_grid):
with pytest.raises(KeyError):
ag_grid["unknown_args"]

def test_underlying_id_is_auto_generated(self, standard_ag_grid):
ag_grid = vm.AgGrid(id="text_ag_grid", figure=standard_ag_grid)
ag_grid.pre_build()
# ag_grid() is the same as ag_grid.__call__()
assert ag_grid().id == "__input_text_ag_grid"

def test_underlying_id_is_provided(self, dash_ag_grid_with_id):
ag_grid = vm.AgGrid(figure=dash_ag_grid_with_id)
ag_grid.pre_build()
# ag_grid() is the same as ag_grid.__call__()
assert ag_grid().id == "underlying_table_id"


class TestAttributesAgGrid:
# Testing at this low implementation level as mocking callback contexts skips checking for creation of these objects
Expand Down Expand Up @@ -141,7 +158,7 @@ def test_ag_grid_build_mandatory_only(self, standard_ag_grid, gapminder):
None,
html.Div(
id="text_ag_grid",
children=dash_ag_grid(data_frame=gapminder, id="__input_text_ag_grid")(),
children=[html.Div(id="__input_text_ag_grid")],
className="table-container",
),
],
Expand All @@ -162,7 +179,7 @@ def test_ag_grid_build_with_underlying_id(self, ag_grid_with_id_and_conf, filter
None,
html.Div(
id="text_ag_grid",
children=dash_ag_grid(data_frame=gapminder, id="underlying_ag_grid_id")(),
children=[html.Div(id="underlying_ag_grid_id")],
className="table-container",
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,8 @@ def test_figure_build(self, standard_kpi_card, gapminder):

expected_figure = dcc.Loading(
html.Div(
kpi_card(
data_frame=gapminder,
value_column="lifeExp",
agg_func="mean",
title="Mean Lifeexp",
value_format="{value:.3f}",
)(),
className="figure-container",
id="figure-id",
className="figure-container",
),
color="grey",
parent_className="loading-container",
Expand Down
Loading
Loading