Fixing subtle bug in the traitlets with This.

Previously subclasses This traitlets wouldn't accept superclass 
instances as values.  I have added a this_class attribute to 
TraitletType that is set by the metaclass and is used later by
This.validate to properly handle this case.

I have also added new tests for this.

IPython/core/component.py

@@ -34,6 +34,9 b' from IPython.utils.traitlets import ('
 #-----------------------------------------------------------------------------
 
 
+class ComponentError(Exception):
+    pass
+
 class MetaComponentTracker(type):
     """A metaclass that tracks instances of Components and its subclasses."""
 
@@ -116,12 +119,12 b' class Component(HasTraitlets):'
     __metaclass__ = MetaComponent
 
     # Traitlets are fun!
-    config = Instance(Struct)
-    parent = This(allow_none=True)
-    root = This(allow_none=True)
+    config = Instance(Struct,(),{})
+    parent = This()
+    root = This()
 
     def __init__(self, parent, name=None, config=None):
-        """Create a component given a parent.
+        """Create a component given a parent and possibly and name and config.
 
         Parameters
         ----------
@@ -132,13 +135,26 b' class Component(HasTraitlets):'
             The unique name of the component.  If empty, then a unique
             one will be autogenerated.
         config : Config
-            If this is empty, self.config = root.config, otherwise
-            self.config = config and root.config is ignored.  This argument 
-            should be used to pass the config to the root.  Otherwise, it
-            can be used to *override* the inheritance of root.config.  If a 
-            caller wants to modify root.config (not override), the caller
-            should make a copy and change attributes and then pass the copy
-            to this argument.  We might think about changing this behavior.
+            If this is empty, self.config = parent.config, otherwise
+            self.config = config and root.config is ignored.  This argument
+            should only be used to *override* the automatic inheritance of 
+            parent.config.  If a caller wants to modify parent.config 
+            (not override), the caller should make a copy and change 
+            attributes and then pass the copy to this argument.
+        
+        Notes
+        -----
+        Subclasses of Component must call the :meth:`__init__` method of
+        :class:`Component` *before* doing anything else and using 
+        :func:`super`::
+        
+            class MyComponent(Component):
+                def __init__(self, parent, name=None, config=None):
+                    super(MyComponent, self).__init__(parent, name, config)
+                    # Then any other code you need to finish initialization.
+
+        This ensures that the :attr:`parent`, :attr:`name` and :attr:`config`
+        attributes are handled properly.
         """
         super(Component, self).__init__()
         self._children = []
@@ -169,6 +185,15 b' class Component(HasTraitlets):'
         else:
             self.root = new.root
 
+    def _root_changed(self, name, old, new):
+        if self.parent is None:
+            if not (new is self):
+                raise ComponentError("Root not self, but parent is None.")
+        else:
+            if not self.parent.root is new:
+                raise ComponentError("Error in setting the root attribute: "
+                                     "root != parent.root")
+        
     @property
     def children(self):
         """A list of all my child components."""

IPython/core/tests/test_component.py

@@ -22,7 +22,11 b' Authors:'
 
 from unittest import TestCase
 
-from IPython.core.component import Component
+from IPython.core.component import Component, ComponentError
+from IPython.utils.traitlets import (
+    TraitletError
+)
+from IPython.utils.ipstruct import Struct
 
 
 #-----------------------------------------------------------------------------
@@ -39,6 +43,30 b' class TestComponentMeta(TestCase):'
         c2 = BaseComponent(c1)
         self.assertEquals(BaseComponent.get_instances(),[c1,c2])
 
+    def test_get_instances_subclass(self):
+        class MyComponent(Component):
+            pass
+        class MyOtherComponent(MyComponent):
+            pass
+        c1 = MyComponent(None)
+        c2 = MyOtherComponent(c1)
+        c3 = MyOtherComponent(c2)
+        self.assertEquals(MyComponent.get_instances(), [c1, c2, c3])
+        self.assertEquals(MyComponent.get_instances(klass=MyOtherComponent), [c2, c3])
+
+    def test_get_instances_root(self):
+        class MyComponent(Component):
+            pass
+        class MyOtherComponent(MyComponent):
+            pass
+        c1 = MyComponent(None)
+        c2 = MyOtherComponent(c1)
+        c3 = MyOtherComponent(c2)
+        c4 = MyComponent(None)
+        c5 = MyComponent(c4)
+        self.assertEquals(MyComponent.get_instances(root=c1), [c1, c2, c3])
+        self.assertEquals(MyComponent.get_instances(root=c4), [c4, c5])
+
 
 class TestComponent(TestCase):
 
@@ -65,3 +93,78 b' class TestComponent(TestCase):'
         self.assertEquals(c2.root, c1)
         self.assertEquals(c3.root, c1)
         self.assertEquals(c4.root, c1)
+
+    def test_change_parent(self):
+        c1 = Component(None)
+        c2 = Component(None)
+        c3 = Component(c1)
+        self.assertEquals(c3.root, c1)
+        self.assertEquals(c3.parent, c1)
+        self.assertEquals(c1.children,[c3])
+        c3.parent = c2
+        self.assertEquals(c3.root, c2)
+        self.assertEquals(c3.parent, c2)
+        self.assertEquals(c2.children,[c3])
+        self.assertEquals(c1.children,[])
+
+    def test_subclass_parent(self):
+        c1 = Component(None)
+        self.assertRaises(TraitletError, setattr, c1, 'parent', 10)
+
+        class MyComponent(Component):
+            pass
+        c1 = Component(None)
+        c2 = MyComponent(c1)
+        self.assertEquals(MyComponent.parent.this_class, Component)
+        self.assertEquals(c2.parent, c1)
+
+    def test_bad_root(self):
+        c1 = Component(None)
+        c2 = Component(None)
+        c3 = Component(None)
+        self.assertRaises(ComponentError, setattr, c1, 'root', c2)
+        c1.parent = c2
+        self.assertEquals(c1.root, c2)
+        self.assertRaises(ComponentError, setattr, c1, 'root', c3)
+
+
+class TestComponentConfig(TestCase):
+
+    def test_default(self):
+        c1 = Component(None)
+        c2 = Component(c1)
+        c3 = Component(c2)
+        self.assertEquals(c1.config, c2.config)
+        self.assertEquals(c2.config, c3.config)
+
+    def test_custom(self):
+        config = Struct()
+        config.FOO = 'foo'
+        config.BAR = 'bar'
+        c1 = Component(None, config=config)
+        c2 = Component(c1)
+        c3 = Component(c2)
+        self.assertEquals(c1.config, config)
+        self.assertEquals(c2.config, config)
+        self.assertEquals(c3.config, config)
+
+class TestComponentName(TestCase):
+
+    def test_default(self):
+        class MyComponent(Component):
+            pass
+        c1 = Component(None)
+        c2 = MyComponent(None)
+        c3 = Component(c2)
+        self.assertNotEquals(c1.name, c2.name)
+        self.assertNotEquals(c1.name, c3.name)
+
+    def test_manual(self):
+        class MyComponent(Component):
+            pass
+        c1 = Component(None, name='foo')
+        c2 = MyComponent(None, name='bar')
+        c3 = Component(c2, name='bah')
+        self.assertEquals(c1.name, 'foo')
+        self.assertEquals(c2.name, 'bar')
+        self.assertEquals(c3.name, 'bah')

IPython/utils/tests/test_traitlets.py

@@ -164,6 +164,17 b' class TestHasTraitletsMeta(TestCase):'
         c.c = 10
         self.assertEquals(c.c,10)
 
+    def test_this_class(self):
+        class A(HasTraitlets):
+            t = This()
+            tt = This()
+        class B(A):
+            tt = This()
+            ttt = This()
+        self.assertEquals(A.t.this_class, A)
+        self.assertEquals(B.t.this_class, A)
+        self.assertEquals(B.tt.this_class, B)
+        self.assertEquals(B.ttt.this_class, B)
 
 class TestHasTraitletsNotify(TestCase):
 
@@ -323,7 +334,7 b' class TestTraitletKeys(TestCase):'
             a = Int
             b = Float
         a = A()
-        self.assertEquals(a.traitlet_keys(),['a','b'])
+        self.assertEquals(a.traitlet_names(),['a','b'])
 
 
 #-----------------------------------------------------------------------------
@@ -484,6 +495,28 b' class TestThis(TestCase):'
         f.this = Foo()
         self.assert_(isinstance(f.this, Foo))
 
+    def test_subclass(self):
+        class Foo(HasTraitlets):
+            t = This()
+        class Bar(Foo):
+            pass
+        f = Foo()
+        b = Bar()
+        f.t = b
+        b.t = f
+        self.assertEquals(f.t, b)
+        self.assertEquals(b.t, f)
+
+    def test_subclass_override(self):
+        class Foo(HasTraitlets):
+            t = This()
+        class Bar(Foo):
+            t = This()
+        f = Foo()
+        b = Bar()
+        f.t = b
+        self.assertEquals(f.t, b)
+        self.assertRaises(TraitletError, setattr, b, 't', f)
 
 class TraitletTestBase(TestCase):
     """A best testing class for basic traitlet types."""

IPython/utils/traitlets.py

@@ -139,6 +139,23 b' def parse_notifier_name(name):'
 
 
 class TraitletType(object):
+    """A base class for all traitlet descriptors.
+
+    Notes
+    -----
+    Our implementation of traitlets is based on Python's descriptor
+    prototol.  This class is the base class for all such descriptors.  The
+    only magic we use is a custom metaclass for the main :class:`HasTraitlets`
+    class that does the following:
+
+    1. Sets the :attr:`name` attribute of every :class:`TraitletType`
+       instance in the class dict to the name of the attribute.
+    2. Sets the :attr:`this_class` attribute of every :class:`TraitletType`
+       instance in the class dict to the *class* that declared the traitlet.
+       This is used by the :class:`This` traitlet to allow subclasses to
+       accept superclasses for :class:`This` values.
+    """
+    
 
     metadata = {}
     default_value = Undefined
@@ -235,6 +252,17 b' class MetaHasTraitlets(type):'
     """
     
     def __new__(mcls, name, bases, classdict):
+        """Create the HasTraitlets class.
+        
+        This instantiates all TraitletTypes in the class dict and sets their
+        :attr:`name` attribute.
+        """
+        # print "========================="
+        # print "MetaHasTraitlets.__new__"
+        # print "mcls, ", mcls
+        # print "name, ", name
+        # print "bases, ", bases
+        # print "classdict, ", classdict
         for k,v in classdict.iteritems():
             if isinstance(v, TraitletType):
                 v.name = k
@@ -245,6 +273,22 b' class MetaHasTraitlets(type):'
                     classdict[k] = vinst
         return super(MetaHasTraitlets, mcls).__new__(mcls, name, bases, classdict)
 
+    def __init__(cls, name, bases, classdict):
+        """Finish initializing the HasTraitlets class.
+        
+        This sets the :attr:`this_class` attribute of each TraitletType in the
+        class dict to the newly created class ``cls``.
+        """
+        # print "========================="
+        # print "MetaHasTraitlets.__init__"
+        # print "cls, ", cls
+        # print "name, ", name
+        # print "bases, ", bases
+        # print "classdict, ", classdict
+        for k, v in classdict.iteritems():
+            if isinstance(v, TraitletType):
+                v.this_class = cls
+        super(MetaHasTraitlets, cls).__init__(name, bases, classdict)
 
 class HasTraitlets(object):
 
@@ -364,7 +408,7 b' class HasTraitlets(object):'
             for n in names:
                 self._add_notifiers(handler, n)
 
-    def traitlet_keys(self):
+    def traitlet_names(self):
         """Get a list of all the names of this classes traitlets."""
         return [memb[0] for memb in inspect.getmembers(self.__class__) if isinstance(memb[1], TraitletType)]
 
@@ -566,7 +610,10 b' class This(ClassBasedTraitletType):'
         super(This, self).__init__(None, **metadata)
 
     def validate(self, obj, value):
-        if isinstance(value, obj.__class__) or (value is None):
+        # What if value is a superclass of obj.__class__?  This is
+        # complicated if it was the superclass that defined the This
+        # traitlet.
+        if isinstance(value, self.this_class) or (value is None):
             return value
         else:
             self.error(obj, value)