Skip to content

Commit

Permalink
Change thinking on optional arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
antonymilne committed Oct 16, 2023
1 parent 44685d9 commit 004451d
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 6 deletions.
3 changes: 2 additions & 1 deletion vizro-core/src/vizro/models/_navigation/_accordion.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class Accordion(VizroBaseModel):
# currently accepts List[str] too
# don't want to get all pages here.
# Make it non-optional for now anyway. Doesn't make sense as optional given now don't have Dropdown etc.
# (technically breaking)
# And there's only one field here anyway.
# (technically breaking if it was public)
pages: Optional[NavigationPagesType] = None
# 2 cases: list of pages, dictionary of pages
# Convert both in validator to dict.
Expand Down
6 changes: 1 addition & 5 deletions vizro-core/src/vizro/models/_navigation/navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ class Navigation(VizroBaseModel):
pages (Optional[NavigationPagesType]): See [`NavigationPagesType`][vizro.models.types.NavigationPagesType].
Defaults to `None`.
"""
# Make pages non-optional here? Probably yes even though technically breaking.
# Could have check that you need to set pages OR selector.

pages: Optional[NavigationPagesType] = None
_selector: Accordion = PrivateAttr()

# validators
_validate_pages = validator("pages", allow_reuse=True, always=True)(_validate_pages)
# does this set to all pages also?

@_log_call
def pre_build(self):
Expand All @@ -78,8 +76,6 @@ def _set_selector(self):
from vizro.models._navigation._accordion import Accordion

# Put in pre_build directly.

# if self.selector is None?
self._selector = Accordion(pages=self.pages)

@_log_call
Expand Down

0 comments on commit 004451d

Please sign in to comment.