enthought / traits

Observable typed attributes for Python classes
Other
438 stars 85 forks source link

Difficulties with Traits initialization order: discussion. #238

Open mdickinson opened 9 years ago

mdickinson commented 9 years ago

Difficulties with Traits initialization order are a recurring problem, that I think have cost many of us time in the past. I'm opening this issue to serve as a place to discuss possible workarounds / solutions / documentation fixes related to this problem. Ultimately, I'd like to be able to at least add recommendations to the documentation for working around this problem, but any solution which makes Traits users less likely to fall into common traps seems worth investigating.

One common pattern is that we have HasTraits subclasses A and B, and that B has an Instance(A) trait where the A instance is expected to be provided at B creation time, and is not expected to change for the lifetime of the B instance. A natural example occurs when B represents some kind of view on a model class A. Here's a concrete example:

from traits.api import Float, HasStrictTraits, Instance, Property

class Model(HasStrictTraits):
    intensity = Float(520.0)

class View(HasStrictTraits):
    model = Instance(Model)
    rescale_factor = Float(1.0)
    rescaled_intensity = Property(Float, depends_on='rescale_factor')

    def _get_rescaled_intensity(self):
        return self.model.intensity * self.rescale_factor

    def _rescaled_intensity_changed(self, old, new):
        print "Rescaled intensity changed from {!r} to {!r}".format(old, new)

if __name__ == '__main__':
    model = Model()
    view = View(model=model, rescale_factor=0.5)

What happens when this code is run? Well, depending on the order in which the View traits are set at initialization time, you'll either get the expected output:

Rescaled intensity changed from None to 260.0

or you'll get a traceback:

Exception occurred in traits notification handler.
Please check the log file for details.
Exception occurred in traits notification handler for object: <__main__.View object at 0x107c742f0>, trait: rescale_factor, old value: 1.0, new value: 0.5
Traceback (most recent call last):
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traits/trait_notifiers.py", line 520, in _dispatch_change_event
    self.dispatch( handler, *args )
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traits/trait_notifiers.py", line 483, in dispatch
    handler( *args )
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traits/has_traits.py", line 966, in wrapper0
    return function(arg)
  File "/Users/mdickinson/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/traits/has_traits.py", line 3310, in notify
    self.trait_property_changed( name, None )
  File "traits_init.py", line 17, in _get_rescaled_intensity
    return self.model.intensity * self.rescale_factor
AttributeError: 'NoneType' object has no attribute 'intensity'

If you run the code on Python 3 (or on Python 2 under PYTHONHASHSEED=random), you'll see both of these behaviours occurring. Otherwise, you'll only see one. It might even be the 'correct' one, but that's fragile: any future change to Python's internal hashing algorithms could turn the 'working' code into failing code.

Two possible patterns for working around this problem:

(1) Treat the model trait as though it could change at any moment: with this solution, None is considered a valid and expected value for self.model, any code that accesses self.model should be prepared to deal with the situation where self.model is None appropriately, and there should be trait change handlers reacting to any change in self.model. In the above example, it's enough to declare the rescaled_intensity trait as depending on self.model, and to have the _get_rescaled_intensity method explicitly check for a model of None and return some dummy value in that case.

(2) Insist that the model trait is supplied at View creation time, and doesn't change for the lifetime of the view object. That involves adding an explicit __init__ method:

    def __init__(self, model, **traits):
        self.model = model
        super(View, self).__init__(**traits)

For extra safety, one can raise an exception on model change:

    def _model_default(self):
        raise ValueError("The model trait should always be provided at creation time.")

    @on_trait_change('model')
    def _raise_on_model_change(self, obj, name, old, new):
        raise ValueError("Leave me alone!")

Note that using a _model_changed method (instead of an on_trait_change-decorated method) doesn't work here: that method causes the _model_default method to be called.

One option I'd like to look into is adding Traits support for this particular pattern: e.g., a piece of metadata that can be used to annotate a trait to indicate that (1) that trait must be provided at creation time, (2) that trait should be initialized before others, and (3) any attempt to change the trait value will produce an exception.

itziakos commented 9 years ago

(3) any attempt to change the trait value will produce an exception.

This feature is currently possible using the Readonly trait but it would be nice if Readonly supported type checking.

cfarrow commented 9 years ago

I don't think of this problem much, as you've seen from my code, but when it comes up I go with solution (2). I encounter this more when I have a DelegatesTo that needs the delegate in place before the traits machinery kicks in. My impression is that solution (2) is discouraged, but I do not know the reasoning behind that.

I think we could do a lot to make (2) easier. E.g. the following could do the __init__ juggling automatically. (I'm not recommending this interface, I'm just showing how easy we could make this.)

class View(HasStrictTraits):
    model = Instance(Model, required=True)
    rescale_factor = Float(1.0)
stefanoborini commented 9 years ago

I think this particular issue, and more generally a lack of documentation on some corner cases and special uses of traits, are biting me quite often. I am willing to document these things as I spot them, but first I have to understand them myself.

kitchoi commented 4 years ago

The documentation may also need to mention that this behaviour depends on the Python versions. In particular, Python 3.7+ guarantees insertion order for dictionary such that View(model=model, rescale_factor=0.5) will initialize model first (no errors), whereas View(rescale_factor=0.5, model=model) will initialize rescale_factor first, causing the exception to occur.