diff --git a/pyproject.toml b/pyproject.toml index 4a649fa9b..1845df386 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,18 +38,18 @@ warn_unused_ignores = true disable_error_code = "name-defined" module = "vizro.models.types" -[tool.pytest.ini_options] -addopts = [ - # Allow test files to have the same name in different directories. - "--import-mode=importlib", -] - [tool.pydantic-mypy] init_forbid_extra = true init_typed = true warn_required_dynamic_aliases = true warn_untyped_fields = true +[tool.pytest.ini_options] +addopts = [ + # Allow test files to have the same name in different directories. + "--import-mode=importlib" +] + [tool.ruff] # see: https://beta.ruff.rs/docs/rules/ ignore = [ diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index b97a1443d..b75b006fd 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -15,6 +15,8 @@ from vizro.actions._action_loop._action_loop import ActionLoop from vizro.models import Navigation, VizroBaseModel from vizro.models._models_utils import _log_call +from vizro.models._navigation._navigation_utils import _NavBuildType +from vizro.models._page import _PageBuildType if TYPE_CHECKING: from vizro.models import Page @@ -101,12 +103,12 @@ def _make_page_layout(self, page: Page): # Shared across pages but slightly differ in content. These could possibly be done by a clientside # callback instead. page_title = html.H2(children=page.title, id="page_title") - navigation = cast(Navigation, self.navigation).build(active_page_id=page.id) + navigation: _NavBuildType = cast(Navigation, self.navigation).build(active_page_id=page.id) nav_bar = navigation["nav_bar_outer"] nav_panel = navigation["nav_panel_outer"] # Different across pages - page_content = page.build() + page_content: _PageBuildType = page.build() control_panel = page_content["control_panel_outer"] component_container = page_content["component_container_outer"] diff --git a/vizro-core/src/vizro/models/_navigation/_navigation_utils.py b/vizro-core/src/vizro/models/_navigation/_navigation_utils.py index 2a058ae4a..1d89eb50e 100644 --- a/vizro-core/src/vizro/models/_navigation/_navigation_utils.py +++ b/vizro-core/src/vizro/models/_navigation/_navigation_utils.py @@ -1,4 +1,9 @@ +from __future__ import annotations + import itertools +from typing import TypedDict + +from dash import html from vizro.managers import model_manager @@ -19,3 +24,12 @@ def _validate_pages(pages): if unknown_pages := [page for page in pages_as_list if page not in registered_pages]: raise ValueError(f"Unknown page ID {unknown_pages} provided to argument 'pages'.") return pages + + +# This is just used for type checking. Ideally it would inherit from some dash.development.base_component.Component +# (e.g. html.Div) as well as TypedDict, but that's not possible, and Dash does not have typing support anyway. When +# this type is used, the object is actually still a dash.development.base_component.Component, but this makes it easier +# to see what contract the component fulfils by making the expected keys explicit. +class _NavBuildType(TypedDict): + nav_bar_outer: html.Div + nav_panel_outer: html.Div diff --git a/vizro-core/src/vizro/models/_navigation/accordion.py b/vizro-core/src/vizro/models/_navigation/accordion.py index 23543e3fd..00cf8d19b 100644 --- a/vizro-core/src/vizro/models/_navigation/accordion.py +++ b/vizro-core/src/vizro/models/_navigation/accordion.py @@ -39,6 +39,7 @@ def coerce_pages_type(cls, pages): @_log_call def build(self, *, active_page_id=None): + # Note this does not return _NavBuildType but just a single html.Div with id="nav_panel_outer". # Hide navigation panel if there is only one page if len(list(itertools.chain(*self.pages.values()))) == 1: return html.Div(hidden=True, id="nav_panel_outer") diff --git a/vizro-core/src/vizro/models/_navigation/nav_bar.py b/vizro-core/src/vizro/models/_navigation/nav_bar.py index c1a11c9a0..32485578c 100644 --- a/vizro-core/src/vizro/models/_navigation/nav_bar.py +++ b/vizro-core/src/vizro/models/_navigation/nav_bar.py @@ -8,7 +8,7 @@ from vizro.models import VizroBaseModel from vizro.models._models_utils import _log_call -from vizro.models._navigation._navigation_utils import _validate_pages +from vizro.models._navigation._navigation_utils import _validate_pages, _NavBuildType from vizro.models._navigation.nav_link import NavLink @@ -56,7 +56,7 @@ def pre_build(self): return self.items @_log_call - def build(self, *, active_page_id=None): + def build(self, *, active_page_id=None) -> _NavBuildType: # We always show all the nav_link buttons, but only show the accordion for the active page. This works because # item.build only returns the nav_panel_outer Div when the item is active. # In future maybe we should do this by showing all navigation panels and then setting hidden=True for all but diff --git a/vizro-core/src/vizro/models/_navigation/navigation.py b/vizro-core/src/vizro/models/_navigation/navigation.py index ee42a3b8b..446bf559d 100644 --- a/vizro-core/src/vizro/models/_navigation/navigation.py +++ b/vizro-core/src/vizro/models/_navigation/navigation.py @@ -5,7 +5,7 @@ from vizro.models import VizroBaseModel from vizro.models._models_utils import _log_call -from vizro.models._navigation._navigation_utils import _validate_pages +from vizro.models._navigation._navigation_utils import _validate_pages, _NavBuildType from vizro.models._navigation.accordion import Accordion from vizro.models.types import NavPagesType, NavSelectorType @@ -37,7 +37,7 @@ def pre_build(self): # AM: test if remove the above do tests break? @_log_call - def build(self, *, active_page_id=None): + def build(self, *, active_page_id=None) -> _NavBuildType: nav_selector = self.nav_selector.build(active_page_id=active_page_id) if "nav_bar_outer" not in nav_selector: # e.g. nav_selector is Accordion and nav_selector.build returns single html.Div with id="nav_panel_outer". diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 64430ccd4..da84cf9da 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import List, Optional +from typing import List, Optional, TypedDict from dash import Input, Output, Patch, callback, dcc, html from pydantic import Field, root_validator, validator @@ -15,6 +15,15 @@ from .types import ComponentType, ControlType +# This is just used for type checking. Ideally it would inherit from some dash.development.base_component.Component +# (e.g. html.Div) as well as TypedDict, but that's not possible, and Dash does not have typing support anyway. When +# this type is used, the object is actually still a dash.development.base_component.Component, but this makes it easier +# to see what contract the component fulfils by making the expected keys explicit. +class _PageBuildType(TypedDict): + control_panel_outer: html.Div + component_container_outer: html.Div + + class Page(VizroBaseModel): """A page in [`Dashboard`][vizro.models.Dashboard] with its own URL path and place in the `Navigation`. @@ -111,7 +120,7 @@ def pre_build(self): ] @_log_call - def build(self): + def build(self) -> _PageBuildType: self._update_graph_theme() controls_content = [control.build() for control in self.controls] control_panel = ( diff --git a/vizro-core/tests/integration/test_navigation.py b/vizro-core/tests/integration/test_navigation.py index 9c0751f7a..d097d4a34 100644 --- a/vizro-core/tests/integration/test_navigation.py +++ b/vizro-core/tests/integration/test_navigation.py @@ -8,17 +8,12 @@ from vizro import Vizro from vizro.managers import model_manager -# Options: -# subclass dict the right way, alter __getitem__, assert function (could be registered), just assert in test -# AM: add more commment about what tests are doing and whether to reuse helper functions - def strip_ids(object): """Strips all entries with key "id" from a dictionary, regardless of how deeply it's nested. This makes it easy to compare dictionaries generated from Dash components we've created that contain random IDs. """ - ... if isinstance(object, dict): object = {key: strip_ids(value) for key, value in object.items() if key != "id"} elif isinstance(object, list): @@ -26,6 +21,10 @@ def strip_ids(object): return object +# We could use this helper more generally but unit tests that currently do +# json.loads(json.dumps(component, cls=plotly.utils.PlotlyJSONEncoder)) feel quite fragile so might be changed anyway. +# If we do want to reuse it more generally, consider whether we should use pytest hooks to do a custom assert or +# convert dictionary to a custom type (e.g. with a special __getitem__ that leaves out "id"). def component_to_dict(component: dash.development.base_component.Component): dictionary = json.loads(json.dumps(component, cls=plotly.utils.PlotlyJSONEncoder)) dictionary = strip_ids(dictionary) @@ -56,6 +55,8 @@ def dashboard_result(request): # All the cases need to built lazily - instantiating them directly would not raise validation errors that the # specified page cannot be found in the page registry. +# In all test cases, the first lambda should be thought of as the "input" configuration, and the second is the +# configuration that results after we've filled in default values etc. # fmt: off accordion_cases = [ ( @@ -166,8 +167,6 @@ def label_cases(cases, label): navbar_flat_cases = label_cases(navbar_flat_cases, "navbar_flat") navbar_grouped_cases = label_cases(navbar_grouped_cases, "navbar_grouped") -print(accordion_cases + navbar_grouped_cases + navbar_flat_cases) - @pytest.mark.parametrize( "dashboard_result, dashboard_expected",