Why a new class?

[zerline]

Hi
Could you explain the logic behind this code:
(in traitlets.HasTraits)
def add_traits(self, **traits): """Dynamically add trait attributes to the HasTraits instance.""" self.__class__ = type(self.__class__.__name__, (self.__class__,), traits)

Why do you need to change the object class?

from ipywidgets import Box
import traitlets
b = Box()
b.class
<class 'ipywidgets.widgets.widget_box.Box'>
b.add_traits(**{'mytrait':traitlets.Unicode()})
b.class
<class 'traitlets.traitlets.Box'>

Many thanks

[rmorshea]

@Zerlin We require that logic because modifying the traits of self.__class__ without reassigning it will impact other instances of the same class. The following demonstrates the side effects of modifying classes:

class MyClass(object):

    y = 1

    def add(self, x):
        return x + self.y


a = MyClass()

assert a.add(1) == 2

b = MyClass()
b.__class__.y = 10
assert b.add(1) == 11

# now here's the kicker...
assert a.add(1) == 11

The above is true because a.__class__ is b.__class__. The fact that traits are class attributes ultimately means that we must dynamically reassign a new __class__ so that when we add traits to one instance we don't accidentally modify the behavior of others.

[zerline]

@rmorshea And what happens if you add one trait, then another? Different instances, with different traits, will have the same class, then. And the __mro__ is somewhat bizarre, with N times 'traitlets.traitlets.Box' ...

And anyway, it looks to me, as a Jupyter user, that different instances of the same widget anyway impact each other. Is it not a desired behaviour?

(please tell me if I my understanding is wrong!)

Regards,

[rmorshea]

Hi @zerline,

To your first question, you are partially correct. It is not true that calling add_traits creates a new "instance", however it is true, that the "class" of that instance get's switched. The following would demonstrate this fact:

cls_1 = obj.__class__
obj.add_traits(**traits)
cls_2 = obj.__class__
assert cls_1 is not cls_2

You are correct though, that if you call add_traits N times, you're __mro__ will contain N more classes. If you do this frequently it will have performance impacts since classes take a large amount of memory and time to construct. For this reason I would highly suggest avoiding the use of add_traits if you can (perhaps you should create a new class instead of adding traits). If you absolutely need to add_traits on the fly, then reduce the number of times you call it.

Concerning widgets, I am not aware of a case where the use of one widget instance impacts the behavior of another from the same class. Can you provide an example where this occurs? There are cases where two widget instances have been explicitly linked via a link object however without seeing a specific case, its hard to tell what might be causing this. If I can't explain what's happening it may be a bug.

[rmorshea]

@minrk have you come across a case where you truly need add_traits? As it stands, users may be temped to dynamically add_traits to HasTraits instances upon creation rather than defining a new class which inherits from a parent and contains the new traits they require. In other words, should add_traits be deprecated?

[zerline]

@rmorshea @minrk Sorry : I still don't understand why you need this class switch. See my previous comment:

...it looks to me, as a Jupyter user, that different instances of the same widget anyway impact each other. Is it not a desired behaviour?

I make intensive use of add_traits for adding traits after instance creation in my sage-combinat-widgets. For example, if a math object is been changed. I also need to remove traits dynamically.

I could use a traitlets.Dict instead. Of course only when it becomes in effect mutable ..

[rmorshea]

@zerline can you provide an example where different instances of the same widget "impact each other"? There are ways that different widget instances can be explicitly linked, though I'm unsure of whether that's really what you're experiencing.

[zerline]

Hi @rmorshea
Was I thinking of different instances, or different displays of the same instance? Probably the latter.
So I might understand better why this class switch was imagined.
Yet that does not seem a very good coding, and it doesn't help when you add traits more than once, because this class name switch happens only the first time.
Could you provide an example of a situation for which this class switch is in effect useful?

[rmorshea]

@zerline I suspect that you were thinking of displays of the same instance, because to my knowledge there is no case where an action taken on one widget instance impacts another instance of the same class unless you manually link them.

Why Switch The Class?

In order for me to explain why we must switch classes you should already understand the following:

1. class attributes vs instance attributes
2. descriptors

Given the above knowledge we can consider that:

1. traitlets.TraitType (the trait base class) is a descriptor and is thus a class attribute.
2. Changes made to one HasTraits instance must not effect other instances of the same class.

It should then follow that we cannot:

1. Add traits to a HasTraits instance.
   • Because traits are class attributes.
2. Modify the instances original __class__.
   • Because that would effect other instances of the same class.

A Concession

I will concede that switching the class more than once is redundant, since after the first switch no other instance would be effected when modifying the new __class__. However I don't think this was part of your original criticism.

[zerline]

Thank you @rmorshea for this explanation.
Now:

1. if this renaming process is necessary, then letting the user choose the new class name would help.
2. I am still confused about all that, because my tests seem to show that adding traits actually do not add class attributes. It seems they add instance attributes:
   from ipywidgets import Text
w1 = Text('Original #1')
w1.add_traits(**{'new':1})
w1.new
   1
   w2 = Text('Original #2')
w2.new
   AttributeError: 'Text' object has no attribute 'new'

[rmorshea]

@zerline

1. I don't think letting the user choose the new class name is really appropriate. This would make add_traits even more confusing than it already is. When you examine the type of your widget after you call add_traits it will appear to be a completely different class even though the it's essentially the same (just with added traits).
2. I would suggest that you read into descriptors. It may help you understand why traits are class attributes even though they appear to belong to instances. I'll approximate how traits are implemented with an example (that assumes Python 3.6):

class TraitType(object):

    def __set_name__(self, owner, name):
        # see PEP 487 for info on this method.
        self.name = name

    # the methods below are standard "descriptor" methods.

    def __get__(self, instance, cls):
        value =  instance._trait_values[self.name]
        print("got %r from %r on %r" % (value, self.name, instance))
        return value

    def __set__(self, instance, value):
        print("setting %r on %r to %r" % (self.name, instance, value))
        instance._trait_values[self.name] = value


class HasTraits(object):

    def __init__(self):
        self._trait_values = {}

    def __repr__(self):
        # this is just here to make pretty printing
        tag = hex(id(self))
        name = self.__class__.__name__
        return "%s(%s)" % (name, tag)


class MyClass(HasTraits):
    # define some traits - note that these are class attributes
    a = TraitType()
    b = TraitType()


mc1 = MyClass()
mc2 = MyClass()

print("first instance\n--------------")
mc1.a = 1
mc1.a
print(mc1.__dict__)

print("\nsecond instance\n---------------")
mc2.b = 785
mc2.b
print(mc2.__dict__)

Expected output:

first instance
--------------
setting 'a' on MyClass(0x7fe2818d1a20) to 1
got 1 from 'a' on MyClass(0x7fe2818d1a20)
{'_trait_values': {'a': 1}}

second instance
---------------
setting 'b' on MyClass(0x7fe280403550) to 785
got 785 from 'b' on MyClass(0x7fe280403550)
{'_trait_values': {'b': 785}}

Note that in both instances of MyClass that a and b are absent from the __dict__. Instead, we find the _trait_values dictionary with the keys a and b instead.

[zerline]

@rmorshea

[ 0) I have read about descriptors, thank you. So you don't need to copy here simplified code so that I better understand, thank you. ]

1. As you (more or less) agree: « new class name ... make add_traits ... confusing ». For me, changing the class module is what is the most confusing. (in my example, from ipywidgets.widgets.widget_box.Box to traitlets.traitlets.Box). Especially if other code expects your container to be of a certain type. And changing it N times is a second, possibly harmful, side effect.

2. If adding a trait to an instance does not really impact other ones, then, to me, this class renaming is not necessary. (your simplified code, here, adds grist to my mill!)

This issue now looks more like a discussion that should take place in a mailing list (maybe there should be one?), So I will just try to summarize here:

I wish there would be no class renaming in add_traits, at least not mandatory.

[rmorshea]

@zerline Just to close things out though, I think I see your confusion about the class renaming now. The naming difference will be sorted out if we set:

new_class.__module__ = original_class.__module__

At the moment we only set:

new_class.__name__ = original_class.__name__`.

Following this change we would retain the full name ipywidgets.widgets.widget_box.Box

The repr for the class is drawing from both its __module__ and its __name__. We could get even more thorough by setting the __qualname__ from the original class as well.

Would that resolve things for you?

Feel free to bring this up in the mailing list if you'd rather continue this there. I'm not subscribed to it so I don't really know how active it is.

[zerline]

Hi sorry for the late response (I was ill, and awaiting a reaction from @minrk).
Sure, it will help a lot!
Although I still don't understand why this mechanics is needed in the first place - let us put this aside for the moment, probably a too partial understanding on my side.
The mro becomes quickly huge, maybe not a problem in itself.
Thank you @rmorshea. This will help me developing sage-combinat-widgets.