Remove Optional from a few places (#210)

mckinsey · Dec 13, 2023 · 06a5da0 · 06a5da0
1 parent 864bafe
commit 06a5da0
Show file tree

Hide file tree

Showing 8 changed files with 90 additions and 41 deletions.
diff --git a/vizro-core/changelog.d/20231213_093615_antony.milne_typing.md b/vizro-core/changelog.d/20231213_093615_antony.milne_typing.md
@@ -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
+
+- 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))
+
+-->
diff --git a/vizro-core/schemas/0.1.7.dev1.json b/vizro-core/schemas/0.1.7.dev1.json
@@ -1,6 +1,6 @@
 {
   "title": "Dashboard",
-  "description": "Vizro Dashboard to be used within [`Vizro`][vizro._vizro.Vizro.build].\n\nArgs:\n    pages (List[Page]): See [`Page`][vizro.models.Page].\n    theme (Literal[\"vizro_dark\", \"vizro_light\"]): Layout theme to be applied across dashboard.\n        Defaults to `vizro_dark`.\n    navigation (Optional[Navigation]): See [`Navigation`][vizro.models.Navigation]. Defaults to `None`.\n    title (str): Dashboard title to appear on every page on top left-side. Defaults to `\"\"`.",
+  "description": "Vizro Dashboard to be used within [`Vizro`][vizro._vizro.Vizro.build].\n\nArgs:\n    pages (List[Page]): See [`Page`][vizro.models.Page].\n    theme (Literal[\"vizro_dark\", \"vizro_light\"]): Layout theme to be applied across dashboard.\n        Defaults to `vizro_dark`.\n    navigation (Navigation): See [`Navigation`][vizro.models.Navigation]. Defaults to `None`.\n    title (str): Dashboard title to appear on every page on top left-side. Defaults to `\"\"`.",
   "type": "object",
   "properties": {
     "id": {
@@ -680,7 +680,7 @@
     },
     "Filter": {
       "title": "Filter",
-      "description": "Filter the data supplied to `targets` on the [`Page`][vizro.models.Page].\n\nExamples:\n    >>> print(repr(Filter(column=\"species\")))\n\nArgs:\n    type (Literal[\"filter\"]): Defaults to `\"filter\"`.\n    column (str): Column of `DataFrame` to filter.\n    targets (List[ModelID]): Target component to be affected by filter. If none are given then target all components\n        on the page that use `column`.\n    selector (Optional[SelectorType]): See [SelectorType][vizro.models.types.SelectorType]. Defaults to `None`.",
+      "description": "Filter the data supplied to `targets` on the [`Page`][vizro.models.Page].\n\nExamples:\n    >>> print(repr(Filter(column=\"species\")))\n\nArgs:\n    type (Literal[\"filter\"]): Defaults to `\"filter\"`.\n    column (str): Column of `DataFrame` to filter.\n    targets (List[ModelID]): Target component to be affected by filter. If none are given then target all components\n        on the page that use `column`.\n    selector (SelectorType): See [SelectorType][vizro.models.types.SelectorType]. Defaults to `None`.",
       "type": "object",
       "properties": {
         "id": {
@@ -711,7 +711,18 @@
         },
         "selector": {
           "title": "Selector",
-          "anyOf": [
+          "description": "Selectors to be used inside a control.",
+          "discriminator": {
+            "propertyName": "type",
+            "mapping": {
+              "checklist": "#/definitions/Checklist",
+              "dropdown": "#/definitions/Dropdown",
+              "radio_items": "#/definitions/RadioItems",
+              "range_slider": "#/definitions/RangeSlider",
+              "slider": "#/definitions/Slider"
+            }
+          },
+          "oneOf": [
             {
               "$ref": "#/definitions/Checklist"
             },
@@ -834,7 +845,7 @@
     },
     "Page": {
       "title": "Page",
-      "description": "A page in [`Dashboard`][vizro.models.Dashboard] with its own URL path and place in the `Navigation`.\n\nArgs:\n    components (List[ComponentType]): See [ComponentType][vizro.models.types.ComponentType]. At least one component\n        has to be provided.\n    title (str): Title to be displayed.\n    layout (Optional[Layout]): Layout to place components in. Defaults to `None`.\n    controls (List[ControlType]): See [ControlType][vizro.models.types.ControlType]. Defaults to `[]`.\n    path (str): Path to navigate to page. Defaults to `\"\"`.\n\nRaises:\n    ValueError: If number of page and grid components is not the same",
+      "description": "A page in [`Dashboard`][vizro.models.Dashboard] with its own URL path and place in the `Navigation`.\n\nArgs:\n    components (List[ComponentType]): See [ComponentType][vizro.models.types.ComponentType]. At least one component\n        has to be provided.\n    title (str): Title to be displayed.\n    layout (Layout): Layout to place components in. Defaults to `None`.\n    controls (List[ControlType]): See [ControlType][vizro.models.types.ControlType]. Defaults to `[]`.\n    path (str): Path to navigate to page. Defaults to `\"\"`.\n\nRaises:\n    ValueError: If number of page and grid components is not the same",
       "type": "object",
       "properties": {
         "id": {

diff --git a/vizro-core/src/vizro/models/_components/_form.py b/vizro-core/src/vizro/models/_components/_form.py
@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, List, Literal, Optional
+from typing import TYPE_CHECKING, List, Literal
 
 from dash import html
 
@@ -31,12 +31,12 @@ class Form(VizroBaseModel):
     Args:
         type (Literal["form"]): Defaults to `"form"`.
         components (List[FormComponentType]): List of components used in the form.
-        layout (Optional[Layout]): Defaults to `None`.
+        layout (Layout): Defaults to `None`.
     """
 
     type: Literal["form"] = "form"
     components: List[_FormComponentType]
-    layout: Optional[Layout] = None
+    layout: Layout = None  # type: ignore[assignment]
 
     @validator("layout", always=True)
     def set_layout(cls, layout, values):
@@ -74,20 +74,18 @@ def build(self):
                     "gridRow": f"{grid_coord.row_start}/{grid_coord.row_end}",
                 },
             )
-            for component, grid_coord in zip(
-                self.components, self.layout.component_grid_lines  # type: ignore[union-attr]
-            )
+            for component, grid_coord in zip(self.components, self.layout.component_grid_lines)
         ]
         return self._make_form_layout(component_container)
 
     def _make_form_layout(self, component_container):
         return html.Div(
             component_container,
             style={
-                "gridRowGap": self.layout.row_gap,  # type: ignore[union-attr]
-                "gridColumnGap": self.layout.col_gap,  # type: ignore[union-attr]
-                "gridTemplateColumns": f"repeat({len(self.layout.grid[0])}, minmax({self.layout.col_min_width}, 1fr))",  # type: ignore[union-attr]  # noqa: E501
-                "gridTemplateRows": f"repeat({len(self.layout.grid)}, minmax({self.layout.row_min_height}, 1fr))",  # type: ignore[union-attr]  # noqa: E501
+                "gridRowGap": self.layout.row_gap,
+                "gridColumnGap": self.layout.col_gap,
+                "gridTemplateColumns": f"repeat({len(self.layout.grid[0])}, minmax({self.layout.col_min_width}, 1fr))",  # noqa: E501
+                "gridTemplateRows": f"repeat({len(self.layout.grid)}, minmax({self.layout.row_min_height}, 1fr))",  # noqa: E501
             },
             className="component_container_grid",
             id=self.id,

diff --git a/vizro-core/src/vizro/models/_components/form/_form_utils.py b/vizro-core/src/vizro/models/_components/form/_form_utils.py
@@ -14,6 +14,7 @@ def get_options_and_default(options: OptionsType, multi: bool = False):
             options = [ALL_OPTION, *options]
 
     if all(isinstance(option, dict) for option in options):
+        # Each option is a OptionsDictType
         default_value = options[0]["value"]  # type: ignore[index]
     else:
         default_value = options[0]

diff --git a/vizro-core/src/vizro/models/_controls/filter.py b/vizro-core/src/vizro/models/_controls/filter.py
@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, List, Literal, Optional
+from typing import TYPE_CHECKING, List, Literal
 
 import pandas as pd
 from pandas.api.types import is_numeric_dtype
@@ -64,7 +64,7 @@ class Filter(VizroBaseModel):
         column (str): Column of `DataFrame` to filter.
         targets (List[ModelID]): Target component to be affected by filter. If none are given then target all components
             on the page that use `column`.
-        selector (Optional[SelectorType]): See [SelectorType][vizro.models.types.SelectorType]. Defaults to `None`.
+        selector (SelectorType): See [SelectorType][vizro.models.types.SelectorType]. Defaults to `None`.
     """
 
     type: Literal["filter"] = "filter"
@@ -74,8 +74,8 @@ class Filter(VizroBaseModel):
         description="Target component to be affected by filter. "
         "If none are given then target all components on the page that use `column`.",
     )
-    selector: Optional[SelectorType] = None
-    _column_type: Optional[Literal["numerical", "categorical"]] = PrivateAttr()
+    selector: SelectorType = None
+    _column_type: Literal["numerical", "categorical"] = PrivateAttr()
 
     @validator("targets", each_item=True)
     def check_target_present(cls, target):
@@ -94,7 +94,7 @@ def pre_build(self):
 
     @_log_call
     def build(self):
-        return self.selector.build()  # type: ignore[union-attr]
+        return self.selector.build()
 
     def _set_targets(self):
         if not self.targets:
@@ -114,11 +114,10 @@ def _set_column_type(self):
             self._column_type = "categorical"
 
     def _set_selector(self):
-        self.selector = self.selector or SELECTOR_DEFAULTS[self._column_type]()  # type: ignore[index]
+        self.selector = self.selector or SELECTOR_DEFAULTS[self._column_type]()
         self.selector.title = self.selector.title or self.column.title()
 
     def _set_slider_values(self):
-        self.selector: SelectorType
         if isinstance(self.selector, SELECTORS["numerical"]):
             if self._column_type != "numerical":
                 raise ValueError(
@@ -142,7 +141,6 @@ def _set_slider_values(self):
                 self.selector.max = max(max_values)
 
     def _set_categorical_selectors_options(self):
-        self.selector: SelectorType
         if isinstance(self.selector, SELECTORS["categorical"]) and not self.selector.options:
             options = set()
             for target_id in self.targets:
@@ -152,7 +150,6 @@ def _set_categorical_selectors_options(self):
             self.selector.options = sorted(options)
 
     def _set_actions(self):
-        self.selector: SelectorType
         if not self.selector.actions:
             filter_function = _filter_between if isinstance(self.selector, RangeSlider) else _filter_isin
             self.selector.actions = [

diff --git a/vizro-core/src/vizro/models/_controls/parameter.py b/vizro-core/src/vizro/models/_controls/parameter.py
@@ -80,25 +80,21 @@ def build(self):
         return self.selector.build()
 
     def _set_slider_values(self):
-        self.selector: SelectorType
         if isinstance(self.selector, (Slider, RangeSlider)):
             if self.selector.min is None or self.selector.max is None:
                 raise TypeError(
                     f"{self.selector.type} requires the arguments 'min' and 'max' when used within Parameter."
                 )
 
     def _set_categorical_selectors_options(self):
-        self.selector: SelectorType
         if isinstance(self.selector, (Checklist, Dropdown, RadioItems)) and not self.selector.options:
             raise TypeError(f"{self.selector.type} requires the argument 'options' when used within Parameter.")
 
     def _set_selector(self):
-        self.selector: SelectorType
         if not self.selector.title:
             self.selector.title = ", ".join({target.rsplit(".")[-1] for target in self.targets})
 
     def _set_actions(self):
-        self.selector: SelectorType
         if not self.selector.actions:
             self.selector.actions = [
                 Action(

diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py
@@ -2,7 +2,7 @@
 
 import logging
 from functools import partial
-from typing import TYPE_CHECKING, List, Literal, Optional, cast
+from typing import TYPE_CHECKING, List, Literal
 
 import dash
 import dash_bootstrap_components as dbc
@@ -35,15 +35,15 @@ class Dashboard(VizroBaseModel):
         pages (List[Page]): See [`Page`][vizro.models.Page].
         theme (Literal["vizro_dark", "vizro_light"]): Layout theme to be applied across dashboard.
             Defaults to `vizro_dark`.
-        navigation (Optional[Navigation]): See [`Navigation`][vizro.models.Navigation]. Defaults to `None`.
+        navigation (Navigation): See [`Navigation`][vizro.models.Navigation]. Defaults to `None`.
         title (str): Dashboard title to appear on every page on top left-side. Defaults to `""`.
     """
 
     pages: List[Page]
     theme: Literal["vizro_dark", "vizro_light"] = Field(
         "vizro_dark", description="Layout theme to be applied across dashboard. Defaults to `vizro_dark`"
     )
-    navigation: Optional[Navigation] = None
+    navigation: Navigation = None  # type: ignore[assignment]
     title: str = Field("", description="Dashboard title to appear on every page on top left-side.")
 
     @validator("pages", always=True)
@@ -109,7 +109,7 @@ 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: _NavBuildType = cast(Navigation, self.navigation).build(active_page_id=page.id)
+        navigation: _NavBuildType = self.navigation.build(active_page_id=page.id)
         nav_bar = navigation["nav_bar_outer"]
         nav_panel = navigation["nav_panel_outer"]
 

diff --git 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, TypedDict
+from typing import List, TypedDict
 
 from dash import Input, Output, Patch, callback, dcc, html
 
@@ -35,7 +35,7 @@ class Page(VizroBaseModel):
         components (List[ComponentType]): See [ComponentType][vizro.models.types.ComponentType]. At least one component
             has to be provided.
         title (str): Title to be displayed.
-        layout (Optional[Layout]): Layout to place components in. Defaults to `None`.
+        layout (Layout): Layout to place components in. Defaults to `None`.
         controls (List[ControlType]): See [ControlType][vizro.models.types.ControlType]. Defaults to `[]`.
         path (str): Path to navigate to page. Defaults to `""`.
 
@@ -45,7 +45,7 @@ class Page(VizroBaseModel):
 
     components: List[ComponentType]
     title: str = Field(..., description="Title to be displayed.")
-    layout: Optional[Layout] = None
+    layout: Layout = None  # type: ignore[assignment]
     controls: List[ControlType] = []
     path: str = Field("", description="Path to navigate to page.")
 
@@ -140,9 +140,7 @@ def build(self) -> _PageBuildType:
                     "gridRow": f"{grid_coord.row_start}/{grid_coord.row_end}",
                 },
             )
-            for component, grid_coord in zip(
-                self.components, self.layout.component_grid_lines  # type: ignore[union-attr]
-            )
+            for component, grid_coord in zip(self.components, self.layout.component_grid_lines)
         ]
         components_container = self._create_component_container(components_content)
         return html.Div([control_panel, components_container])
@@ -178,11 +176,11 @@ def _create_component_container(self, components_content):
                 html.Div(
                     components_content,
                     style={
-                        "gridRowGap": self.layout.row_gap,  # type: ignore[union-attr]
-                        "gridColumnGap": self.layout.col_gap,  # type: ignore[union-attr]
-                        "gridTemplateColumns": f"repeat({len(self.layout.grid[0])},"  # type: ignore[union-attr]
+                        "gridRowGap": self.layout.row_gap,
+                        "gridColumnGap": self.layout.col_gap,
+                        "gridTemplateColumns": f"repeat({len(self.layout.grid[0])},"
                         f"minmax({self.layout.col_min_width}, 1fr))",
-                        "gridTemplateRows": f"repeat({len(self.layout.grid)},"  # type: ignore[union-attr]
+                        "gridTemplateRows": f"repeat({len(self.layout.grid)},"
                         f"minmax({self.layout.row_min_height}, 1fr))",
                     },
                     className="component_container_grid",