From 06a5da01f05207c0cf64487a28a81b05be975a46 Mon Sep 17 00:00:00 2001 From: Antony Milne <49395058+antonymilne@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:35:27 +0000 Subject: [PATCH] Remove `Optional` from a few places (#210) --- .../20231213_093615_antony.milne_typing.md | 48 +++++++++++++++++++ vizro-core/schemas/0.1.7.dev1.json | 19 ++++++-- .../src/vizro/models/_components/_form.py | 18 ++++--- .../models/_components/form/_form_utils.py | 1 + .../src/vizro/models/_controls/filter.py | 15 +++--- .../src/vizro/models/_controls/parameter.py | 4 -- vizro-core/src/vizro/models/_dashboard.py | 8 ++-- vizro-core/src/vizro/models/_page.py | 18 ++++--- 8 files changed, 90 insertions(+), 41 deletions(-) create mode 100644 vizro-core/changelog.d/20231213_093615_antony.milne_typing.md diff --git a/vizro-core/changelog.d/20231213_093615_antony.milne_typing.md b/vizro-core/changelog.d/20231213_093615_antony.milne_typing.md new file mode 100644 index 000000000..f1f65e73c --- /dev/null +++ b/vizro-core/changelog.d/20231213_093615_antony.milne_typing.md @@ -0,0 +1,48 @@ + + + + + + + + + diff --git a/vizro-core/schemas/0.1.7.dev1.json b/vizro-core/schemas/0.1.7.dev1.json index 9af909add..4d1e6947f 100644 --- 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 index 1ef9b44a6..cbf9b7142 100644 --- 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,9 +74,7 @@ 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) @@ -84,10 +82,10 @@ 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 index 1780ec583..f6af61ffd 100644 --- 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 index 01ceb239e..cc4f52e4d 100644 --- 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 index be5f1b3c0..b7c04c294 100644 --- a/vizro-core/src/vizro/models/_controls/parameter.py +++ b/vizro-core/src/vizro/models/_controls/parameter.py @@ -80,7 +80,6 @@ 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( @@ -88,17 +87,14 @@ def _set_slider_values(self): ) 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 index c1d78c4b3..6c2bc0c9c 100644 --- 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,7 +35,7 @@ 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 `""`. """ @@ -43,7 +43,7 @@ class Dashboard(VizroBaseModel): 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 index 9391fa49c..e4e9a5f04 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, 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",