Repository: ipython/traitlets
Pull request #415 (2017-06-06 11:45:52)

follow mro for trait default generators
adds a regression test.

This appears to have been broken by the new default value generators, which don't resolve correctly on the mro.

From the looks of it, `_trait_default_generators` on classes should not include inherited generators, and lookups should instead walk the dicts on the mro every time.

cc @rmorshea


Patch:
diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py
index 57d81ce..03b7f6d 100644
--- a/traitlets/traitlets.py
+++ b/traitlets/traitlets.py
@@ -490,7 +490,7 @@ class TraitType(BaseDescriptor):
             value = obj._trait_values[self.name]
         except KeyError:
             # Check for a dynamic initializer.
-            default = cls._trait_default_generators[self.name](obj)
+            default = obj.trait_defaults(self.name)
             if default is Undefined:
                 raise TraitError("No default value found for "
                     "the '%s' trait named '%s' of %r" % (
@@ -776,11 +776,6 @@ class MetaHasTraits(MetaHasDescriptors):
     def setup_class(cls, classdict):
         cls._trait_default_generators = {}
         super(MetaHasTraits, cls).setup_class(classdict)
-        new = {}
-        for c in reversed(cls.mro()):
-            if hasattr(c, "_trait_default_generators"):
-                new.update(c._trait_default_generators)
-        cls._trait_default_generators = new
 
 
 
@@ -1460,6 +1455,17 @@ class HasTraits(six.with_metaclass(MetaHasTraits, HasDescriptors)):
         """
         return {name: getattr(self, name) for name in self.trait_names(**metadata)}
 
+    @classmethod
+    def _get_trait_default_generator(cls, name):
+        """Return default generator for a given trait
+
+        Walk the MRO to resolve the correct default generator according to inheritance.
+        """
+        for c in cls.mro():
+            if name in c.__dict__.get('_trait_default_generators', {}):
+                return c._trait_default_generators[name]
+        raise KeyError("No default generator for trait %r found in %r" % (name, cls.mro()))
+
     def trait_defaults(self, *names, **metadata):
         """Return a trait's default value or a dictionary of them
 
@@ -1468,7 +1474,7 @@ class HasTraits(six.with_metaclass(MetaHasTraits, HasDescriptors)):
         Dynamically generated default values may
         depend on the current state of the object."""
         if len(names) == 1 and len(metadata) == 0:
-            return self._trait_default_generators[names[0]](self)
+            return self._get_trait_default_generator(names[0])(self)
 
         for n in names:
             if not has_trait(self, n):
@@ -1479,7 +1485,7 @@ class HasTraits(six.with_metaclass(MetaHasTraits, HasDescriptors)):
 
         defaults = {}
         for n in trait_names:
-            defaults[n] = self._trait_default_generators[n](self)
+            defaults[n] = self._get_trait_default_generator(n)(self)
         return defaults
 
     def trait_names(self, **metadata):
@@ -1743,11 +1749,13 @@ class Instance(ClassBasedTraitType):
 
     def info(self):
         if isinstance(self.klass, six.string_types):
-            result = add_article(self.klass)
+            klass = self.klass
         else:
-            result = describe("a", self.klass)
+            klass = self.klass.__name__
+        result = class_of(klass)
         if self.allow_none:
-            result += ' or None'
+            return result + ' or None'
+
         return result
 
     def instance_init(self, obj):


Test patch:
diff --git a/traitlets/tests/test_traitlets.py b/traitlets/tests/test_traitlets.py
index 3a81e88..2d9665b 100644
--- a/traitlets/tests/test_traitlets.py
+++ b/traitlets/tests/test_traitlets.py
@@ -2496,6 +2496,39 @@ def test_super_bad_args():
     assert not hasattr(obj, 'b')
 
 
+def test_default_mro():
+    """Verify that default values follow mro"""
+    class Base(HasTraits):
+        trait = Unicode('base')
+        attr = 'base'
+
+    class A(Base):
+        pass
+
+    class B(Base):
+        trait = Unicode('B')
+        attr = 'B'
+
+    class AB(A, B):
+        pass
+
+    class BA(B, A):
+        pass
+
+    assert 'trait' in Base._trait_default_generators
+    assert 'trait' not in A._trait_default_generators
+    assert 'trait' in B._trait_default_generators
+    assert 'trait' not in AB._trait_default_generators
+    assert 'trait' not in BA._trait_default_generators
+
+    assert A().trait == 'base'
+    assert A().attr == 'base'
+    assert BA().trait == 'B'
+    assert BA().attr == 'B'
+    assert AB().trait == 'B'
+    assert AB().attr == 'B'
+
+
 def test_cls_self_argument():
     class X(HasTraits):
         def __init__(__self, cls, self):