diff --git a/tests/test_traitlets.py b/tests/test_traitlets.py index 2e7b8459..5daab4bb 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/argcomplete_config.py b/traitlets/config/argcomplete_config.py index 1a411cec..7444a765 100644 --- a/traitlets/config/argcomplete_config.py +++ b/traitlets/config/argcomplete_config.py @@ -45,7 +45,7 @@ def get_argcomplete_cwords() -> list[str] | None: _cword_suffix, comp_words, _last_wordbreak_pos, - ) = argcomplete.split_line(comp_line, comp_point) # type:ignore[attr-defined,no-untyped-call] + ) = argcomplete.split_line(comp_line, comp_point) # type:ignore[attr-defined] except ModuleNotFoundError: return None @@ -73,7 +73,7 @@ def increment_argcomplete_index() -> None: os.environ["_ARGCOMPLETE"] = str(int(os.environ["_ARGCOMPLETE"]) + 1) except Exception: try: - argcomplete.debug("Unable to increment $_ARGCOMPLETE", os.environ["_ARGCOMPLETE"]) # type:ignore[attr-defined,no-untyped-call] + argcomplete.debug("Unable to increment $_ARGCOMPLETE", os.environ["_ARGCOMPLETE"]) # type:ignore[attr-defined] except (KeyError, ModuleNotFoundError): pass @@ -194,7 +194,7 @@ def _get_completions(self, comp_words: list[str], cword_prefix: str, *args: t.An # Instead, check if comp_words only consists of the script, # if so check if any subcommands start with cword_prefix. if self.subcommands and len(comp_words) == 1: - argcomplete.debug("Adding subcommands for", cword_prefix) # type:ignore[attr-defined,no-untyped-call] + argcomplete.debug("Adding subcommands for", cword_prefix) # type:ignore[attr-defined] completions.extend(subc for subc in self.subcommands if subc.startswith(cword_prefix)) return completions diff --git a/traitlets/config/loader.py b/traitlets/config/loader.py index 876f1186..379cd479 100644 --- a/traitlets/config/loader.py +++ b/traitlets/config/loader.py @@ -1133,7 +1133,7 @@ def _argcomplete(self, classes: list[t.Any], subcommands: SubcommandsDict | None from . import argcomplete_config - finder = argcomplete_config.ExtendedCompletionFinder() # type:ignore[no-untyped-call] + finder = argcomplete_config.ExtendedCompletionFinder() finder.config_classes = classes finder.subcommands = list(subcommands or []) # for ease of testing, pass through self._argcomplete_kwargs if set diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py index 0989ea98..0fc3ae8a 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,19 @@ 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 + # The fast loop above already ran validate() and stored each + # kwarg. The second pass is only needed for traits with a + # cross-validator: _cross_validate may change the value, which + # set_trait then persists and notifies. Without one, + # _cross_validate is a passthrough and set_trait would just run + # validate() a second time on the unchanged value for nothing, + # so record the already-stored value and skip that work. + 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 +1823,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 +1999,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."""