Add type hints

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 = [

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"]
 

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

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")

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

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".

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 = (

vizro-core/tests/integration/test_navigation.py

@@ -8,24 +8,23 @@
 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):
         object = [strip_ids(item) for item in 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",