From 9443d56ecbcd00cf0d9ab86357f743b5f7b4206a Mon Sep 17 00:00:00 2001 From: Antony Milne Date: Mon, 2 Dec 2024 17:17:53 +0000 Subject: [PATCH] Fix merge --- vizro-core/src/vizro/_vizro.py | 5 ++--- .../_callback_mapping_utils.py | 4 ++-- vizro-core/src/vizro/managers/_model_manager.py | 6 ++++-- vizro-core/src/vizro/models/_page.py | 17 ++++++++++------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/vizro-core/src/vizro/_vizro.py b/vizro-core/src/vizro/_vizro.py index 81c9a46b7..ad530be00 100644 --- a/vizro-core/src/vizro/_vizro.py +++ b/vizro-core/src/vizro/_vizro.py @@ -145,13 +145,12 @@ def _pre_build(): # Any models that are created during the pre-build process *will not* themselves have pre_build run on them. # In future may add a second pre_build loop after the first one. - # model_manager results is wrapped into a list to avoid RuntimeError: dictionary changed size during iteration - for _, filter_obj in list(model_manager._items_with_type(Filter)): + for filter in model_manager._get_models(Filter): # Run pre_build on all filters first, then on all other models. This handles dependency between Filter # and Page pre_build and ensures that filters are pre-built before the Page objects that use them. # This is important because the Page pre_build method checks whether filters are dynamic or not, which is # defined in the filter's pre_build method. - filter_obj.pre_build() + filter.pre_build() for model_id in set(model_manager): model = model_manager[model_id] if hasattr(model, "pre_build") and not isinstance(model, Filter): diff --git a/vizro-core/src/vizro/actions/_callback_mapping/_callback_mapping_utils.py b/vizro-core/src/vizro/actions/_callback_mapping/_callback_mapping_utils.py index 79aa19ba7..f275f570d 100644 --- a/vizro-core/src/vizro/actions/_callback_mapping/_callback_mapping_utils.py +++ b/vizro-core/src/vizro/actions/_callback_mapping/_callback_mapping_utils.py @@ -7,8 +7,8 @@ from vizro.actions import _parameter, filter_interaction from vizro.managers import model_manager -from vizro.managers._model_manager import ModelID, FIGURE_MODELS -from vizro.models import Action, AgGrid, Figure, Graph, Page, Table, VizroBaseModel +from vizro.managers._model_manager import FIGURE_MODELS, ModelID +from vizro.models import Action, Page, VizroBaseModel from vizro.models._action._actions_chain import ActionsChain from vizro.models._controls import Filter, Parameter from vizro.models.types import ControlType diff --git a/vizro-core/src/vizro/managers/_model_manager.py b/vizro-core/src/vizro/managers/_model_manager.py index bebf2b6e1..b260eb111 100644 --- a/vizro-core/src/vizro/managers/_model_manager.py +++ b/vizro-core/src/vizro/managers/_model_manager.py @@ -76,14 +76,16 @@ def _get_models( model_type = (vm.Graph, vm.AgGrid, vm.Table, vm.Figure) models = self.__get_model_children(page) if page is not None else self.__models.values() - for model in models: + # Convert to list to avoid changing size when looping through at runtime. + for model in list(models): if model_type is None or isinstance(model, model_type): yield model def __get_model_children(self, model: Model) -> Generator[Model, None, None]: """Iterates through children of `model`. - Currently looks only through certain fields so might miss some children models.""" + Currently looks only through certain fields so might miss some children models. + """ from vizro.models import VizroBaseModel if isinstance(model, VizroBaseModel): diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 508adee2f..9e91c35a8 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Mapping -from typing import Any, Optional, TypedDict, Union +from typing import Any, Optional, TypedDict, Union, cast, Iterable from dash import dcc, html @@ -13,8 +13,8 @@ from vizro._constants import ON_PAGE_LOAD_ACTION_PREFIX from vizro.actions import _on_page_load from vizro.managers import model_manager -from vizro.managers._model_manager import DuplicateIDError, FIGURE_MODELS -from vizro.models import Action, AgGrid, Figure, Graph, Layout, Table, VizroBaseModel +from vizro.managers._model_manager import FIGURE_MODELS, DuplicateIDError +from vizro.models import Action, Layout, VizroBaseModel, Filter from vizro.models._action._actions_chain import ActionsChain, Trigger from vizro.models._layout import set_layout from vizro.models._models_utils import _log_call, check_captured_callable, validate_min_length @@ -96,10 +96,13 @@ def __vizro_exclude_fields__(self) -> Optional[Union[set[str], Mapping[str, Any] @_log_call def pre_build(self): - targets = model_manager._get_page_model_ids_with_figure(page_id=ModelID(str(self.id))) - - # TODO NEXT: make work generically for control group - targets.extend(control.id for control in self.controls if getattr(control, "_dynamic", False)) + figure_targets = [model.id for model in model_manager._get_models(FIGURE_MODELS, page=self)] + filter_targets = [ + filter.id + for filter in cast(Iterable[Filter], model_manager._get_models(Filter, page=self)) + if filter._dynamic + ] + targets = figure_targets + filter_targets if targets: self.actions = [