From 43521cfcd36eeb9e15a64bf009e536729f74da7b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 12:20:07 +0200 Subject: [PATCH 1/4] Fix class_own_trait_events calling nonexistent cls.events() HasTraits.class_own_trait_events called the nonexistent cls.events() and raised AttributeError on every call; the method had clearly never worked (nothing in the test suite exercised it). Call trait_events() instead, and fix the docstring, which pointed at an equally nonexistent ``event_handlers`` method. Also remove the unreachable 'tags' branch in trait_events: it guarded on hasattr(v, "tags"), but no EventHandler ever gets a .tags attribute -- tags are trait metadata, not handler attributes -- so the branch could never run. Add regression tests for trait_events / class_own_trait_events inheritance behavior. Co-Authored-By: Claude Fable 5 --- tests/test_traitlets.py | 38 ++++++++++++++++++++++++++++++++++++++ traitlets/traitlets.py | 11 ++--------- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/tests/test_traitlets.py b/tests/test_traitlets.py index 6d2190fc..ea8cde56 100644 --- a/tests/test_traitlets.py +++ b/tests/test_traitlets.py @@ -836,6 +836,44 @@ class A(HasTraits): self.assertTrue(a.has_trait("f")) self.assertFalse(a.has_trait("g")) + def test_trait_events(self): + class A(HasTraits): + i = Int() + + @observe("i") + def _on_i(self, change): + pass + + class B(A): + f = Float() + + @observe("f") + def _on_f(self, change): + pass + + self.assertEqual(sorted(B.trait_events()), ["_on_f", "_on_i"]) + self.assertEqual(list(B.trait_events("f")), ["_on_f"]) + self.assertEqual(list(B.trait_events("i")), ["_on_i"]) + + def test_class_own_trait_events(self): + class A(HasTraits): + i = Int() + + @observe("i") + def _on_i(self, change): + pass + + class B(A): + f = Float() + + @observe("f") + def _on_f(self, change): + pass + + self.assertEqual(list(A.class_own_trait_events("i")), ["_on_i"]) + self.assertEqual(list(B.class_own_trait_events("f")), ["_on_f"]) + self.assertEqual(list(B.class_own_trait_events("i")), []) + def test_trait_has_value(self): class A(HasTraits): i = Int() diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py index 65bb616d..d149f13c 100644 --- a/traitlets/traitlets.py +++ b/traitlets/traitlets.py @@ -1964,14 +1964,10 @@ def trait_metadata(self, traitname: str, key: str, default: t.Any = None) -> t.A def class_own_trait_events(cls: type[HasTraits], name: str) -> dict[str, EventHandler]: """Get a dict of all event handlers defined on this class, not a parent. - Works like ``event_handlers``, except for excluding traits from parents. + Works like ``trait_events``, except for excluding traits from parents. """ sup = super(cls, cls) - return { - n: e - for (n, e) in cls.events(name).items() # type:ignore[attr-defined] - if getattr(sup, n, None) is not e - } + return {n: e for (n, e) in cls.trait_events(name).items() if getattr(sup, n, None) is not e} @classmethod def trait_events(cls: type[HasTraits], name: str | None = None) -> dict[str, EventHandler]: @@ -1994,9 +1990,6 @@ def trait_events(cls: type[HasTraits], name: str | None = None) -> dict[str, Eve events[k] = v elif name in v.trait_names: # type:ignore[attr-defined] events[k] = v - elif hasattr(v, "tags"): - if cls.trait_names(**v.tags): - events[k] = v return events From eb41a835febb6d7facb9639bf840d82032a7f102 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 12:20:29 +0200 Subject: [PATCH 2/4] Fix flatten_flags checking the wrong variable for tuple aliases Application.flatten_flags checked isinstance(aliases, tuple), i.e. the output dict being built (never a tuple), instead of the loop variable `alias`. mypy had flagged the branch as unreachable and the warning was silenced with a type:ignore rather than investigated. As a result tuple-valued alias keys like ("f", "foo") were never exploded into individual names, so the collision check between flags and aliases missed them, and a flag sharing a name with one member of a tuple alias crashed argparse with 'conflicting option strings' instead of merging into an optional-argument alias. Add regression tests for tuple-alias flattening and flag/alias name collision. Co-Authored-By: Claude Fable 5 --- tests/config/test_application.py | 27 +++++++++++++++++++++++++++ traitlets/config/application.py | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/config/test_application.py b/tests/config/test_application.py index b0720fb8..3ff3581a 100644 --- a/tests/config/test_application.py +++ b/tests/config/test_application.py @@ -510,6 +510,33 @@ def test_flatten_aliases(self): # this would be app.config.Application.log_level if it failed: self.assertEqual(app.config.MyApp.log_level, "CRITICAL") + def test_flatten_aliases_tuple_keys(self): + # tuple alias keys must be exploded into one entry per name, so + # that the loader can detect collisions between flags and aliases + app = MyApp() + flags, aliases = app.flatten_flags() + self.assertEqual(aliases["fooi"], "Foo.i") + self.assertEqual(aliases["i"], "Foo.i") + self.assertNotIn(("fooi", "i"), aliases) + + def test_flag_tuple_alias_collision(self): + # a flag sharing a name with one member of a tuple alias used to + # crash argparse with 'conflicting option strings' + class CollisionApp(Application): + classes = List([Bar]) # type:ignore[assignment] + aliases = {("b", "bee"): "Bar.b"} + flags = {"b": ({"Bar": {"enabled": False}}, "Disable Bar")} + + # with an argument it acts as the alias + app = CollisionApp() + app.parse_command_line(["-b", "5"]) + self.assertEqual(app.config.Bar.b, 5) + + # without an argument it acts as the flag + app = CollisionApp() + app.parse_command_line(["-b"]) + self.assertEqual(app.config.Bar.enabled, False) + def test_extra_args(self): app = MyApp() app.parse_command_line(["--Bar.b=5", "extra", "args", "--disable"]) diff --git a/traitlets/config/application.py b/traitlets/config/application.py index a63dcf35..5b1f0e6a 100644 --- a/traitlets/config/application.py +++ b/traitlets/config/application.py @@ -751,7 +751,7 @@ def flatten_flags(self) -> tuple[dict[str, t.Any], dict[str, t.Any]]: if len(children) == 1: # exactly one descendent, promote alias cls = children[0] # type:ignore[assignment] - if not isinstance(aliases, tuple): # type:ignore[unreachable] + if not isinstance(alias, tuple): # type:ignore[unreachable] alias = (alias,) # type:ignore[assignment] for al in alias: aliases[al] = ".".join([cls, trait]) From 89a73520bf8bfb4f9a79ad23773570146058dcba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 12:20:37 +0200 Subject: [PATCH 3/4] Fix get_logger caching the fallback logger before Application existed get_logger() permanently cached whichever logger it returned first. If it was first called before any Application existed (easy to hit: any config-file load warning does it), the fallback library logger was cached and the Application's configured logger was never returned afterward, silently dropping app log output routed through traitlets.log. Check Application.initialized() on every call instead. With that fix the module-level _logger cache no longer served any purpose -- logging.getLogger() already caches by name -- so remove the mutable global entirely and attach the NullHandler once at import time, per stdlib best practice for libraries. A side benefit of not caching: after Application.clear_instance(), callers fall back to the library logger instead of a stale reference to the torn-down app's. Add a regression test for get_logger picking up an Application created after the first call. Co-Authored-By: Claude Fable 5 --- tests/config/test_application.py | 17 +++++++++++++++++ traitlets/log.py | 25 ++++++++++--------------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/tests/config/test_application.py b/tests/config/test_application.py index 3ff3581a..59b7d1be 100644 --- a/tests/config/test_application.py +++ b/tests/config/test_application.py @@ -969,6 +969,23 @@ def test_logging_teardown_on_error(capsys, caplogconfig): assert len(caplogconfig) == 1 # logging was configured +def test_get_logger_after_application(): + # get_logger must pick up the Application's logger even if it was + # first called (returning the fallback) before the Application existed, + # and fall back again once the Application is torn down + import traitlets.log + + Application.clear_instance() + try: + fallback = traitlets.log.get_logger() + assert fallback.name == "traitlets" + app = Application.instance() + assert traitlets.log.get_logger() is app.log + finally: + Application.clear_instance() + assert traitlets.log.get_logger() is fallback + + if __name__ == "__main__": # for test_help_output: MyApp.launch_instance() diff --git a/traitlets/log.py b/traitlets/log.py index 112b2004..4b8b932f 100644 --- a/traitlets/log.py +++ b/traitlets/log.py @@ -5,27 +5,22 @@ from __future__ import annotations import logging -from typing import Any +from typing import Any, cast -_logger: logging.Logger | logging.LoggerAdapter[Any] | None = None +# Add a NullHandler to silence warnings about not being +# initialized, per best practice for libraries. +_fallback = logging.getLogger("traitlets") +_fallback.addHandler(logging.NullHandler()) def get_logger() -> logging.Logger | logging.LoggerAdapter[Any]: """Grab the global logger instance. If a global Application is instantiated, grab its logger. - Otherwise, grab the root logger. + Otherwise, grab the 'traitlets' library logger. """ - global _logger # noqa: PLW0603 + from .config import Application - if _logger is None: - from .config import Application - - if Application.initialized(): - _logger = Application.instance().log - else: - _logger = logging.getLogger("traitlets") - # Add a NullHandler to silence warnings about not being - # initialized, per best practice for libraries. - _logger.addHandler(logging.NullHandler()) - return _logger + if Application.initialized(): + return cast("logging.Logger | logging.LoggerAdapter[Any]", Application.instance().log) + return _fallback From 7d5e38d9a8ffb20dacfe7cb1f095de3249c9e6c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 12:04:42 +0200 Subject: [PATCH 4/4] Deduplicate TraitTestBase in favor of the shipped copy traitlets/tests/test_traitlets.py and tests/test_traitlets.py each defined their own copy of TraitTestBase. Downstream projects (e.g. ipywidgets, in widgets/tests/test_traits.py) import TraitTestBase from the installed traitlets.tests.test_traitlets module, so that copy has to stay; drop the duplicate from tests/test_traitlets.py and import it from the shipped module instead, and add a docstring noting the downstream dependency so it isn't removed as apparently-unused again. Verified by running ipywidgets 8.1.8's test_traits.py against this tree: 30 passed. Co-Authored-By: Claude Fable 5 --- tests/test_traitlets.py | 54 +------------------------------ traitlets/tests/test_traitlets.py | 6 ++++ 2 files changed, 7 insertions(+), 53 deletions(-) diff --git a/tests/test_traitlets.py b/tests/test_traitlets.py index ea8cde56..d2c755a4 100644 --- a/tests/test_traitlets.py +++ b/tests/test_traitlets.py @@ -64,6 +64,7 @@ traitlets, validate, ) +from traitlets.tests.test_traitlets import TraitTestBase from traitlets.utils import cast_unicode from ._warnings import expected_warnings @@ -1287,59 +1288,6 @@ class Tree(HasTraits): tree.leaves = [1, 2] -class TraitTestBase(TestCase): - """A best testing class for basic trait types.""" - - def assign(self, value): - self.obj.value = value # type:ignore - - def coerce(self, value): - return value - - def test_good_values(self): - if hasattr(self, "_good_values"): - for value in self._good_values: - self.assign(value) - self.assertEqual(self.obj.value, self.coerce(value)) # type:ignore - - def test_bad_values(self): - if hasattr(self, "_bad_values"): - for value in self._bad_values: - try: - self.assertRaises(TraitError, self.assign, value) - except AssertionError: - assert False, value # noqa: PT015 - - def test_default_value(self): - if hasattr(self, "_default_value"): - self.assertEqual(self._default_value, self.obj.value) # type:ignore - - def test_allow_none(self): - if ( - hasattr(self, "_bad_values") - and hasattr(self, "_good_values") - and None in self._bad_values - ): - trait = self.obj.traits()["value"] # type:ignore - try: - trait.allow_none = True - self._bad_values.remove(None) - # skip coerce. Allow None casts None to None. - self.assign(None) - self.assertEqual(self.obj.value, None) # type:ignore - self.test_good_values() - self.test_bad_values() - finally: - # tear down - trait.allow_none = False - self._bad_values.append(None) - - def tearDown(self): - # restore default value after tests, if set - if hasattr(self, "_default_value"): - self.obj.value = self._default_value # type:ignore - - class AnyTrait(HasTraits): value = Any() diff --git a/traitlets/tests/test_traitlets.py b/traitlets/tests/test_traitlets.py index 8380059f..68eb908f 100644 --- a/traitlets/tests/test_traitlets.py +++ b/traitlets/tests/test_traitlets.py @@ -1,3 +1,9 @@ +"""Reusable trait-testing helpers shipped with the package. + +Do not remove: downstream projects (e.g. ipywidgets) import TraitTestBase +from here to test their own trait types. +""" + from __future__ import annotations from typing import Any