Fix three correctness bugs found in code review by claude Fable

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"])
@@ -942,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()

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
@@ -836,6 +837,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()
@@ -1249,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()
 

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])

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

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

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