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] Remove setting of pio.templates.default on import #615

Merged
merged 20 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
13e37d5
Remove pio.templates.default setting and post-update _DashboardReadyF…
antonymilne Aug 2, 2024
69d2167
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 2, 2024
7b1fdd3
Remove pio.templates.default setting and post-update _DashboardReadyF…
antonymilne Aug 2, 2024
62f49fc
Introduce context manager to handle theme in notebook
antonymilne Aug 6, 2024
14fb4d4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 6, 2024
51ccb77
Add ugly tests
antonymilne Aug 6, 2024
a96dfcd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 6, 2024
0abdc98
Merge branch 'main' into tidy/no-more-global-theme
antonymilne Aug 7, 2024
2488c52
Merge branch 'main' into tidy/no-more-global-theme
antonymilne Aug 7, 2024
f342d23
Fix dcc.Store themes
antonymilne Aug 7, 2024
d61fcda
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 7, 2024
6f9d3df
Complete tests
antonymilne Aug 7, 2024
768769b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 7, 2024
053be56
Minor scratch_dev app.py changes
petar-qb Aug 8, 2024
7b9ff58
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 8, 2024
ca2d9f2
Merge branch 'main' into tidy/no-more-global-theme
antonymilne Aug 15, 2024
e463ef7
[Docs] Remove setting of `pio.templates.default` on import (#631)
antonymilne Aug 15, 2024
9439b7d
Merge branch 'main' into tidy/no-more-global-theme
antonymilne Aug 15, 2024
547d8ba
Lint
antonymilne Aug 15, 2024
9272b7a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 15, 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,48 @@
<!--
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
antonymilne marked this conversation as resolved.
Show resolved Hide resolved

- 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

- A bullet item for the Fixed 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))

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

-->
116 changes: 26 additions & 90 deletions vizro-core/examples/scratch_dev/app.py
Original file line number Diff line number Diff line change
@@ -1,113 +1,49 @@
"""Dev app to try things out."""

import plotly.graph_objs as go
import plotly.io as pio
import vizro.models as vm
import vizro.plotly.express as px
from vizro import Vizro
from vizro.figures import kpi_card
from vizro.models.types import capture
from vizro.tables import dash_ag_grid, dash_data_table

df = px.data.iris()


# Graph
@capture("graph")
def my_graph_figure(data_frame, **kwargs):
"""My custom figure."""
return px.scatter(data_frame, **kwargs)


class MyGraph(vm.Graph):
"""My custom class."""

def build(self):
"""Custom build."""
graph_build_obj = super().build()
# DO SOMETHING
return graph_build_obj


# Table
@capture("table")
def my_table_figure(data_frame, **kwargs):
"""My custom figure."""
return dash_data_table(data_frame, **kwargs)()


class MyTable(vm.Table):
"""My custom class."""

pass
# This is applied properly as the final title color is taken from the fig.layout.template
# (which is calculated from the current pio.template)
pio.templates["vizro_dark"]["layout"]["title"]["font"]["color"] = "green"
pio.templates["vizro_light"]["layout"]["title"]["font"]["color"] = "blue"

# This doesn't work as expected as the final scatter points color is taken from fig.data
# (which is calculated probably from the pio.templates.default)
pio.templates["vizro_dark"]["layout"]["colorway"] = ["red"]
petar-qb marked this conversation as resolved.
Show resolved Hide resolved
pio.templates["vizro_light"]["layout"]["colorway"] = ["yellow"]

antonymilne marked this conversation as resolved.
Show resolved Hide resolved
# AgGrid
@capture("ag_grid")
def my_ag_grid_figure(data_frame, **kwargs):
"""My custom figure."""
return dash_ag_grid(data_frame, **kwargs)()


class MyAgGrid(vm.AgGrid):
"""My custom class."""

pass


# Figure
@capture("figure")
def my_kpi_card_figure(data_frame, **kwargs):
"""My custom figure."""
return kpi_card(data_frame, **kwargs)()


class MyFigure(vm.Figure):
"""My custom class."""

pass

df = px.data.iris()

# Action
@capture("action")
def my_action_function():
"""My custom action."""
pass

@capture("graph")
def my_graph_figure_px(data_frame):
"""Blah"""
fig = px.scatter(data_frame, x="sepal_width", y="sepal_length", title="Title")
return fig

class MyAction(vm.Action):
"""My custom class."""

pass
@capture("graph")
def my_graph_figure_go(data_frame):
"""Blahhrl"""
fig = go.Figure(go.Scatter(x=data_frame["sepal_width"], y=data_frame["sepal_length"]))
fig.update_layout(title="Title")
return fig


page = vm.Page(
title="Test",
layout=vm.Layout(
grid=[[0, 1], [2, 3], [4, 5], [6, 7], [8, -1]],
col_gap="50px",
row_gap="50px",
),
components=[
# Graph
MyGraph(figure=px.scatter(df, x="sepal_width", y="sepal_length", title="My Graph")),
MyGraph(figure=my_graph_figure(df, x="sepal_width", y="sepal_length", title="My Graph Custom Figure")),
# Table
MyTable(figure=dash_data_table(df), title="My Table"),
MyTable(figure=my_table_figure(df), title="My Table Custom Figure"),
# AgGrid
MyAgGrid(figure=dash_ag_grid(df), title="My AgGrid"),
MyAgGrid(figure=my_ag_grid_figure(df), title="My AgGrid Custom Figure"),
# Figure
MyFigure(figure=kpi_card(df, value_column="sepal_width", title="KPI Card")),
MyFigure(figure=my_kpi_card_figure(df, value_column="sepal_width", title="KPI Card Custom Figure")),
# Action
MyGraph(
figure=my_graph_figure(df, x="sepal_width", y="sepal_length", title="My Graph Custom Figure"),
actions=[MyAction(function=my_action_function())],
),
vm.Graph(figure=px.scatter(df, x="sepal_width", y="sepal_length", title="Title")),
vm.Graph(figure=my_graph_figure_px(df)),
vm.Graph(figure=my_graph_figure_go(df)),
],
controls=[vm.Filter(column="species")],
)

# Try with theme="vizro_light"
dashboard = vm.Dashboard(pages=[page])

if __name__ == "__main__":
Expand Down
6 changes: 6 additions & 0 deletions vizro-core/src/vizro/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import logging
import os

import plotly.io as pio

from ._themes import dark, light
from ._vizro import Vizro

__all__ = ["Vizro"]

__version__ = "0.1.20.dev0"

logging.basicConfig(level=os.getenv("VIZRO_LOG_LEVEL", "WARNING"))

pio.templates["vizro_dark"] = dark
pio.templates["vizro_light"] = light
11 changes: 11 additions & 0 deletions vizro-core/src/vizro/_vizro.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import dash
import flask
import plotly.io as pio
from flask_caching import SimpleCache

from vizro._constants import STATIC_URL_PREFIX
Expand Down Expand Up @@ -84,6 +85,16 @@ def build(self, dashboard: Dashboard):
if dashboard.title:
self.dash.title = dashboard.title

# Set global template to vizro_light or vizro_dark.
# The choice between these is generally meaningless because chart colours in the two are identical, and
# everything else gets overridden in the post-fig creation layout.template update in Graph.__call__ and the
# clientside theme selector callback.
# Note this setting of global template isn't undone anywhere. If we really wanted to then we could try and
# put in some teardown code, but it would probably never be 100% reliable. Vizro._reset can't do this well
# either because it's a staticmethod. Remember this template setting can't go in run() though since it's needed
# even in deployment.
pio.templates.default = dashboard.theme

# Note that model instantiation and pre_build are independent of Dash.
self._pre_build()

Expand Down
16 changes: 0 additions & 16 deletions vizro-core/src/vizro/charts/_charts_utils.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
import logging

import plotly.io as pio
from plotly import graph_objects as go

from vizro._themes import dark, light

pio.templates["vizro_dark"] = dark
pio.templates["vizro_light"] = light
pio.templates.default = "vizro_dark"

logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)
logger.info(
"Overwriting plotly default template with vizro chart template. "
"To revert back to the default template, run `import plotly.io as pio; pio.templates.default = 'plotly'`."
)
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved


class _DashboardReadyFigure(go.Figure):
# Just for IDE completion and to define new attribute
Expand Down
15 changes: 13 additions & 2 deletions vizro-core/src/vizro/models/_components/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import pandas as pd

from vizro import _themes as themes
from vizro.actions._actions_utils import CallbackTriggerDict, _get_component_actions
from vizro.managers import data_manager, model_manager
from vizro.managers._model_manager import ModelID
Expand Down Expand Up @@ -61,14 +60,26 @@ def __call__(self, **kwargs):
if fig.layout.title.text is None:
fig.update_layout(margin_t=24)

# Apply the template vizro_dark or vizro_light by setting fig.layout.template. This is exactly the same as
# what the clientside update_graph_theme callback does, and it would be nice if we could just use that by
# including Input(self.id, "figure") as input for that callback, but doing so leads to a small flicker between
# completion of this serverside callback and starting that clientside callbacks.
# Note that this does not fully set the template for plotly.express figures. Doing this post-fig creation update
# relies on the fact that we have already set pio.templates.default before the self.figure call
# above (but not with the _pio_templates_default context manager surrounding the above self.figure call,
# since that would alter global state).
# Possibly we should pass through the theme selector as an argument `template` in __call__ rather than fetching
# it from ctx here. Remember that passing it as self.figure(template) is not helpful though, because custom
# graph figures don't need a template argument, and the clientside them selector callback would override this
# anyway.
# Possibly we should enforce that __call__ can only be used within the context of a callback, but it's easy
# to just swallow up the error here as it doesn't cause any problems.
try:
# At the moment theme_selector is always present so this if statement is redundant, but possibly in
# future we'll have callbacks that do Graph.__call__() without theme_selector set.
if "theme_selector" in ctx.args_grouping.get("external", {}):
theme_selector_checked = ctx.args_grouping["external"]["theme_selector"]["value"]
fig["layout"]["template"] = themes.light if theme_selector_checked else themes.dark
fig.layout.template = "vizro_light" if theme_selector_checked else "vizro_dark"
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
except MissingCallbackContextException:
logger.info("fig.update_layout called outside of callback context.")
return fig
Expand Down
6 changes: 4 additions & 2 deletions vizro-core/src/vizro/models/_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import dash
import dash_bootstrap_components as dbc
import dash_mantine_components as dmc
import plotly.io as pio
from dash import (
ClientsideFunction,
Input,
Expand All @@ -28,7 +29,6 @@
from dash.development.base_component import Component

import vizro
from vizro import _themes as themes
from vizro._constants import MODULE_PAGE_404, STATIC_URL_PREFIX
from vizro.actions._action_loop._action_loop import ActionLoop
from vizro.models import Navigation, VizroBaseModel
Expand Down Expand Up @@ -154,7 +154,9 @@ def build(self):
id="dashboard-container",
children=[
html.Div(id="vizro_version", children=vizro.__version__, hidden=True),
dcc.Store(id="vizro_themes", data={"dark": themes.dark, "light": themes.light}),
dcc.Store(
id="vizro_themes", data={"dark": pio.templates["vizro_dark"], "light": pio.templates["vizro_light"]}
),
ActionLoop._create_app_callbacks(),
dash.page_container,
],
Expand Down
52 changes: 51 additions & 1 deletion vizro-core/src/vizro/models/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
import functools
import importlib
import inspect
from contextlib import contextmanager
from datetime import date
from typing import Any, Dict, List, Literal, Protocol, Union, runtime_checkable

import plotly.io as pio

try:
from pydantic.v1 import Field, StrictBool
from pydantic.v1.fields import ModelField
Expand Down Expand Up @@ -248,6 +251,38 @@ def _check_type(cls, captured_callable: CapturedCallable, field: ModelField) ->
return captured_callable


@contextmanager
def _pio_templates_default():
"""Sets pio.templates.default to "vizro_dark" and then reverts it.

This is to ensure that in a Jupyter notebook captured charts look the same as when they're in the dashboard. When
antonymilne marked this conversation as resolved.
Show resolved Hide resolved
the context manager exits the global theme is reverted just to keep things clean (e.g. if you really wanted to,
you could compare a captured vs. non-captured chart in the same Python session).

This works even if users have tweaked the templates, so long as pio.templates has been updated correctly and you
refer to template by name rather than trying to take from vizro.themes.

If pio.templates.default has already been set to vizro_dark or vizro_light then no change is made to allow a user
to set these without it being overridden.
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
"""
old_default = pio.templates.default
template_changed = False
# If the user has set pio.templates.default to a vizro theme already, no need to change it.
if old_default not in ["vizro_dark", "vizro_light"]:
template_changed = True
pio.templates.default = "vizro_dark"

# Revert the template. This is done in a try/finally so that if the code wrapped inside the context manager (i.e.
# plotting functions) raises an exception, pio.templates.default is still reverted. This is not very important
# but easy to achieve.
try:
# This will always be vizro_light or vizro_dark and corresponds to the default theme that has been set.
yield pio.templates.default
finally:
if template_changed:
pio.templates.default = old_default


class capture:
"""Captures a function call to create a [`CapturedCallable`][vizro.models.types.CapturedCallable].

Expand Down Expand Up @@ -332,7 +367,22 @@ def wrapped(*args, **kwargs) -> _DashboardReadyFigure:
fig = _DashboardReadyFigure()
else:
# Standard case for px.scatter(df: pd.DataFrame).
fig = func(*args, **kwargs)
# Set theme for the figure that gets shown in a Jupyter notebook. This is to ensure that in a
# Jupyter notebook captured charts look the same as when they're in the dashboard. To mimic this,
# we first use _pio_templates_default to set the global theme, as is done in the dashboard, and then
# do the fig.layout.template update that is achieved by the theme selector.
# We don't want to update the captured_callable in the same way, since it's only used inside the
# dashboard, at which point the global pio.templates.default is always set anyway according to
# the dashboard theme and then updated according to the theme selector.
with _pio_templates_default() as default_template:
fig = func(*args, **kwargs)
# Update the fig.layout.template just to ensure absolute consistency with how the dashboard
# works.
# The only exception here is the edge case that the user has specified template="vizro_light" or
# "vizro_dark" in the plotting function, in which case we don't want to change it. This makes
# it easier for a user to try out both themes simultaneously in a notebook.
if fig.layout.template not in (pio.templates["vizro_dark"], pio.templates["vizro_light"]):
fig.layout.template = default_template
fig.__class__ = _DashboardReadyFigure

fig._captured_callable = captured_callable
Expand Down
Loading
Loading