diff --git a/tests/test_traitlets.py b/tests/test_traitlets.py index 3e9377fa..db0b399c 100644 --- a/tests/test_traitlets.py +++ b/tests/test_traitlets.py @@ -937,6 +937,42 @@ class A(HasTraits): traits = a.traits(config_key=lambda v: True) self.assertEqual(traits, dict(i=A.i, f=A.f, j=A.j)) + def test_traits_metadata_filter_caching(self): + # metadata-filtered class_traits()/traits() results are memoized per + # class; make sure the cache preserves the "fresh dict" contract and is + # invalidated when metadata is mutated after class creation. + class A(HasTraits): + i = Int().tag(config=True) + j = Int() + + # returned dict is a fresh copy the caller may mutate freely + first = A.class_traits(config=True) + self.assertEqual(first, dict(i=A.i)) + first["injected"] = "oops" + self.assertEqual(A.class_traits(config=True), dict(i=A.i)) + + # tagging a trait after the result was cached must be reflected + A.j.tag(config=True) + self.assertEqual(A.class_traits(config=True), dict(i=A.i, j=A.j)) + self.assertEqual(A().traits(config=True), dict(i=A.i, j=A.j)) + + # a subclass has its own cache and does not pollute the parent's + class B(A): + k = Int().tag(config=True) + + self.assertEqual(B.class_traits(config=True), dict(i=A.i, j=A.j, k=B.k)) + self.assertEqual(A.class_traits(config=True), dict(i=A.i, j=A.j)) + + # filters with non-hashable or callable values bypass the cache without + # error and still filter correctly + self.assertEqual(A.class_traits(config=[1, 2]), {}) # unhashable -> uncached + self.assertEqual(A.class_traits(config=lambda v: v is True), dict(i=A.i, j=A.j)) + + # set_metadata() (deprecated) also invalidates the cache + with expected_warnings([r"Deprecated in traitlets 4.1"]): + A.j.set_metadata("config", False) + self.assertEqual(A.class_traits(config=True), dict(i=A.i)) + def test_traits_metadata_deprecated(self): with expected_warnings([r"metadata should be set using the \.tag\(\) method"] * 2): diff --git a/traitlets/config/configurable.py b/traitlets/config/configurable.py index d2bafef8..d8acfc24 100644 --- a/traitlets/config/configurable.py +++ b/traitlets/config/configurable.py @@ -171,12 +171,15 @@ def _load_config( ) -> None: """load traits from a Config object""" + my_config = self._find_my_config(cfg) + if not my_config: + # Nothing in the config applies to this instance; avoid the cost of + # computing traits() and entering hold_trait_notifications() for the + # many leaf Configurables that carry no matching config. + return + if traits is None: traits = self.traits(config=True) - if section_names is None: - section_names = self.section_names() - - my_config = self._find_my_config(cfg) # hold trait notifications until after all config has been loaded with self.hold_trait_notifications(): diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py index 5e066aee..e5b37072 100644 --- a/traitlets/traitlets.py +++ b/traitlets/traitlets.py @@ -59,6 +59,12 @@ SequenceTypes = (list, tuple, set, frozenset) +# Bumped whenever trait metadata is mutated after class creation (via +# TraitType.tag()/set_metadata()). Used to invalidate the per-class cache of +# metadata-filtered traits kept by HasTraits._traits_matching_metadata. Kept in +# a one-element list so it can be mutated without a module-level `global`. +_trait_metadata_generation = [0] + if t.TYPE_CHECKING: import pathlib @@ -871,6 +877,7 @@ def set_metadata(self, key: str, value: t.Any) -> None: else: msg = "use the instance .metadata dictionary directly, like x.metadata[key] = value" warn("Deprecated in traitlets 4.1, " + msg, DeprecationWarning, stacklevel=2) + _trait_metadata_generation[0] += 1 self.metadata[key] = value def tag(self, **metadata: t.Any) -> Self: @@ -894,6 +901,7 @@ def tag(self, **metadata: t.Any) -> Self: stacklevel=2, ) + _trait_metadata_generation[0] += 1 self.metadata.update(metadata) return self @@ -994,12 +1002,18 @@ def __init__( super().__init__(name, bases, classdict, **kwds) cls.setup_class(classdict) - def setup_class(cls: MetaHasDescriptors, classdict: dict[str, t.Any]) -> None: + def setup_class( + cls: MetaHasDescriptors, classdict: dict[str, t.Any] + ) -> list[tuple[str, t.Any]]: """Setup descriptor instance on the class This sets the :attr:`this_class` and :attr:`name` attributes of each BaseDescriptor in the class dict of the newly created ``cls`` before calling their :attr:`class_init` method. + + Returns the ``getmembers(cls)`` result so that subclass metaclasses + (e.g. :class:`MetaHasTraits`) can reuse it instead of walking the + class namespace a second time. """ cls._descriptors = [] cls._instance_inits: list[t.Any] = [] @@ -1007,35 +1021,34 @@ def setup_class(cls: MetaHasDescriptors, classdict: dict[str, t.Any]) -> None: if isinstance(v, BaseDescriptor): v.class_init(cls, k) # type:ignore[arg-type] - for _, v in getmembers(cls): + members = getmembers(cls) + for _, v in members: if isinstance(v, BaseDescriptor): v.subclass_init(cls) # type:ignore[arg-type] cls._descriptors.append(v) + return members class MetaHasTraits(MetaHasDescriptors): """A metaclass for HasTraits.""" - def setup_class(cls: MetaHasTraits, classdict: dict[str, t.Any]) -> None: + def setup_class(cls: MetaHasTraits, classdict: dict[str, t.Any]) -> list[tuple[str, t.Any]]: # for only the current class cls._trait_default_generators: dict[str, t.Any] = {} # also looking at base classes cls._all_trait_default_generators = {} cls._traits = {} + # per-class cache for metadata-filtered class_traits()/traits() results + cls._traits_metadata_cache: dict[t.Any, tuple[int, dict[str, t.Any]]] = {} cls._static_immutable_initial_values = {} - super().setup_class(classdict) + # Reuse the members collected by the parent metaclass rather than + # walking the whole class namespace (dir(cls) + getattr) a second time. + members = super().setup_class(classdict) mro = cls.mro() - for name in dir(cls): - # Some descriptors raise AttributeError like zope.interface's - # __provides__ attributes even though they exist. This causes - # AttributeErrors even though they are listed in dir(cls). - try: - value = getattr(cls, name) - except AttributeError: - continue + for name, value in members: if isinstance(value, TraitType): cls._traits[name] = value trait = value @@ -1101,6 +1114,8 @@ def setup_class(cls: MetaHasTraits, classdict: dict[str, t.Any]) -> None: # and then the instance may not have all the _static_immutable_initial_values cls._all_trait_default_generators[name] = trait.default + return members + def observe(*names: Sentinel | str, type: str = "change") -> ObserveHandler: """A decorator which can be used to observe Traits on a class. @@ -1334,6 +1349,7 @@ class HasTraits(HasDescriptors, metaclass=MetaHasTraits): _trait_validators: dict[str | Sentinel, t.Any] _cross_validation_lock: bool _traits: dict[str, t.Any] + _traits_metadata_cache: dict[t.Any, tuple[int, dict[str, TraitType[t.Any, t.Any]]]] _all_trait_default_generators: dict[str, t.Any] def setup_instance(self, /, *args: t.Any, **kwargs: t.Any) -> None: @@ -1378,9 +1394,17 @@ def ignore(change: Bunch) -> None: # notify and cross validate all trait changes that were set in kwargs changed = set(kwargs) & set(self._traits) for key in changed: - value = self._traits[key]._cross_validate(self, getattr(self, key)) - self.set_trait(key, value) - changes[key]["new"] = value + # Only re-run the (relatively expensive) cross-validation + + # set_trait pass for traits that actually have a cross-validator. + # For the common case with none, the value stored by the fast + # loop above is already fully validated; we just need to record + # the (possibly coerced) stored value for the notification. + if key in self._trait_validators or hasattr(self, f"_{key}_validate"): + value = self._traits[key]._cross_validate(self, getattr(self, key)) + self.set_trait(key, value) + changes[key]["new"] = value + else: + changes[key]["new"] = getattr(self, key) self._cross_validation_lock = False # Restore method retrieval from class del self.notify_change @@ -1797,21 +1821,64 @@ def class_traits(cls: type[HasTraits], **metadata: t.Any) -> dict[str, TraitType the output. If a metadata key doesn't exist, None will be passed to the function. """ - traits = cls._traits.copy() - if len(metadata) == 0: - return traits + return cls._traits.copy() + + # Return a copy so callers can freely mutate the result; the underlying + # (cached) dict must not escape by reference. + return cls._traits_matching_metadata(metadata).copy() + + @classmethod + def _traits_matching_metadata( + cls: type[HasTraits], metadata: dict[str, t.Any] + ) -> dict[str, TraitType[t.Any, t.Any]]: + """Return the subset of ``cls._traits`` matching a metadata filter. - result = {} - for name, trait in traits.items(): - for meta_name, meta_eval in metadata.items(): - if not callable(meta_eval): - meta_eval = _SimpleTest(meta_eval) + The result is shared, not copied — callers (``class_traits``/``traits``) + are responsible for copying before returning it to user code. + + For filters whose values are all non-callable and hashable (the hot + path, e.g. ``config=True``), the result is memoized per class. Because + ``cls._traits`` is frozen after class creation, the only way the answer + can change is a post-hoc metadata mutation via ``tag()``/``set_metadata()``, + which bump ``_trait_metadata_generation``; cache entries older than the + current generation are recomputed. + """ + # Build a cache key only for constant (non-callable) filters; callable + # predicates are the cold path and are never cached. + key: t.Any = None + if not any(callable(v) for v in metadata.values()): + try: + key = tuple(sorted(metadata.items())) + hash(key) # ensure the values are hashable before use as a key + except TypeError: + key = None + + generation = _trait_metadata_generation[0] + cache: dict[t.Any, tuple[int, dict[str, TraitType[t.Any, t.Any]]]] | None = ( + cls.__dict__.get("_traits_metadata_cache") + ) + if key is not None and cache is not None: + entry = cache.get(key) + if entry is not None and entry[0] == generation: + return entry[1] + + # Normalize the metadata filters once, rather than rebuilding a + # _SimpleTest for every trait on every call. + checks = [ + (meta_name, meta_eval if callable(meta_eval) else _SimpleTest(meta_eval)) + for meta_name, meta_eval in metadata.items() + ] + result: dict[str, TraitType[t.Any, t.Any]] = {} + for name, trait in cls._traits.items(): + for meta_name, meta_eval in checks: if not meta_eval(trait.metadata.get(meta_name, None)): break else: result[name] = trait + if key is not None and cache is not None: + cache[key] = (generation, result) return result @classmethod @@ -1930,22 +1997,12 @@ def traits(self, **metadata: t.Any) -> dict[str, TraitType[t.Any, t.Any]]: the output. If a metadata key doesn't exist, None will be passed to the function. """ - traits = self._traits.copy() - if len(metadata) == 0: - return traits + return self._traits.copy() - result = {} - for name, trait in traits.items(): - for meta_name, meta_eval in metadata.items(): - if not callable(meta_eval): - meta_eval = _SimpleTest(meta_eval) - if not meta_eval(trait.metadata.get(meta_name, None)): - break - else: - result[name] = trait - - return result + # Delegates to the (cached) class-level implementation; self._traits is + # always type(self)._traits. Return a copy so callers can mutate freely. + return type(self)._traits_matching_metadata(metadata).copy() def trait_metadata(self, traitname: str, key: str, default: t.Any = None) -> t.Any: """Get metadata values for trait by key."""