enthought / traits

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

Pickling error with DelegatesTo on multiple levels #549

Open FedeMiorelli opened 4 years ago

FedeMiorelli commented 4 years ago

I have an issue restoring pickled traits instances when I use DelegatesTo in multiple levels. In this example C1 delegates Value to C2 which delegates it to C3. After de-serialization, I'm getting an error: traits.trait_errors.DelegationError: The 'Value' attribute of a 'C2' object has a delegate which does not have traits.

Other than printing the error, it then appears to works fine. Am I doing anything wrong or is there a way to suppress this error?

traits 5.1.2, Python 3.7.4 win10 64 bit

Thanks


import pickle

from traits.api import HasStrictTraits, Str, Any, DelegatesTo, Instance

class C1(HasStrictTraits):
    Parent = Any
    Value = DelegatesTo('Parent')
    def __init__(self, parent):
        HasStrictTraits.__init__(self)
        self.Parent = parent

class C2(HasStrictTraits):
    Parent = Any
    Value = DelegatesTo('Parent')
    Child = Instance(C1)

    def _Child_default(self):
        return C1(self)

    def __init__(self, parent):
        HasStrictTraits.__init__(self)
        self.Parent = parent

class C3(HasStrictTraits):
    Value = Str
    Child = Instance(C2)

    def _Child_default(self):
        return C2(self)

    def save(self):
        with open('test.dump', 'wb') as fid:
            pickle.dump(self.__getstate__(), fid, protocol=2)

    def load(self):
        with open('test.dump', 'rb') as fid:
            self.__setstate__(pickle.load(fid))

i1 = C3()
i1.Value = 'test'
i1.save()

i2 = C3()
i2.load()

print(i2.Value)
corranwebster commented 4 years ago

Thanks for reporting.

My guess (without having run the code) is that during deserialization there is a point in time when C2.Parent is None (the default value for an Any trait) and traits is trying to access C2.Value.

There are a number of mitigations/work arounds that might be possible - for example using Instance(C1, ()) instead of Any may work (untested!) as that avoids a transient None; or writing a custom __setstate__ for one of the classes.

It might also be a bug.

FedeMiorelli commented 4 years ago

@corranwebster Thanks for your feedback. The problem I have is I cannot instance a default value because there is chain of parent relationships which would require me to instantiate a chain of fake instances.. Of course this is just a stripped down version of an actual code which is much more complex.

How would you go about working around this issue in a custom __setstate__ you mentioned?

Thanks

corranwebster commented 4 years ago

I sat down and ran this, and hadn't noticed that the delegation was back up the chain rather than down so there is a reference cycle. I also noticed that you are pickling the state, not the actual object, which means that you are not capturing the entire reference cycle in your pickle - this means that you end up with two C3 instances:

print(i2)
print(i2.Child.Parent)

gives

<__main__.C3 object at 0x116b4b150>
<__main__.C3 object at 0x116b4b2b0>

Note the different ids. So this code is not really working the way that you expect.

So, idiomatically, I would re-write save to pickle the entire object, and turn load into a staticmethod or module-level function:

class C3(HasStrictTraits):
    Value = Str
    Child = Instance(C2)

    def _Child_default(self):
        return C2(self)

    def save(self):
        with open('test.dump', 'wb') as fid:
            pickle.dump(self, fid, protocol=2)

    @staticmethod
    def load():
        with open('test.dump', 'rb') as fid:
            return pickle.load(fid)

i1 = C3()
i1.Value = 'test'
i1.save()

i2 = C3.load()

print(i2.Value)
print(i2)
print(i2.Child.Parent)

which gives output

test
<__main__.C3 object at 0x117c371a8>
<__main__.C3 object at 0x117c371a8>

Note the ids are the same.

I'd also strongly suggest a _child_changed handler for C3 if you don't already have one that sets the parent of the child to the new parent object:

def _child_changed(self, new):
    new.Parent = self

which will help avoid problems with the base C3 object and Child.Parent (although it wouldn't help with the original issue).

FedeMiorelli commented 4 years ago

@corranwebster Thank you for your suggestion, that's really helpful. I had not realized the issue with pickling the state instead of the object and I will change that. I am still getting the reported error though - would you consider that a bug or is it due to an incorrect use?

Cheers, Federico

corranwebster commented 4 years ago

That's curious - I am not seeing any errors when I run the modified code on Python 3.6. Can you please paste the exact code that you are seeing the issue with so that I can verify.

FedeMiorelli commented 4 years ago

Here is the code:


import pickle

from traits.api import HasStrictTraits, Str, Any, DelegatesTo, Instance

class C1(HasStrictTraits):
    Parent = Any
    Value = DelegatesTo('Parent')

    def __init__(self, parent):
        HasStrictTraits.__init__(self)
        self.Parent = parent

class C2(HasStrictTraits):
    Parent = Any
    Value = DelegatesTo('Parent')
    Child = Instance(C1)

    def _Child_default(self):
        return C1(self)

    def _Child_changed(self, new):
        new.Parent = self

    def __init__(self, parent):
        HasStrictTraits.__init__(self)
        self.Parent = parent

class C3(HasStrictTraits):
    Value = Str
    Child = Instance(C2)

    def _Child_default(self):
        return C2(self)

    def _Child_changed(self, new):
        new.Parent = self

    def save(self):
        with open('test.dump', 'wb') as fid:
            pickle.dump(self, fid, protocol=2)

    @staticmethod
    def load():
        with open('test.dump', 'rb') as fid:
            return pickle.load(fid)

i1 = C3()
i1.Value = 'test'
i1.save()

i2 = C3.load()
print(i2.Value)

And I'm getting this output - last line is the expected "test" print.


Python 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 20:34:20) on Windows (64 bits).
This is the Pyzo interpreter.
Type 'help' for help, type '?' for a list of *magic* commands.
Running script: "C:\temp\test.py"
Exception occurred in traits notification handler.
Please check the log file for details.
Exception occurred in traits notification handler for object: <__main__.C1 object at 0x000001CAB85A9B28>, trait: Parent, old value: None, new value: <__main__.C2 object at 0x000001CAB85A9AC8>
Traceback (most recent call last):
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\trait_notifiers.py", line 654, in _dispatch_change_event
    self.dispatch(handler, *args)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\trait_notifiers.py", line 553, in dispatch
    handler(*args)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\traits_listener.py", line 498, in handle_simple
    self.next.register(new)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\traits_listener.py", line 425, in register
    trait = new.base_trait(name)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\has_traits.py", line 3157, in base_trait
    return self._trait(name, -2)
traits.trait_errors.DelegationError: The 'Value' attribute of a 'C2' object has a delegate which does not have traits.
Exception occurred in traits notification handler for object: <__main__.C1 object at 0x000001CAB85A9B28>, trait: Parent, old value: None, new value: <__main__.C2 object at 0x000001CAB85A9AC8>
Traceback (most recent call last):
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\trait_notifiers.py", line 654, in _dispatch_change_event
    self.dispatch(handler, *args)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\trait_notifiers.py", line 553, in dispatch
    handler(*args)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\traits_listener.py", line 498, in handle_simple
    self.next.register(new)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\traits_listener.py", line 425, in register
    trait = new.base_trait(name)
  File "c:\winpython-64bit-3.7.4.1\python-3.7.4.amd64\lib\site-packages\traits\has_traits.py", line 3157, in base_trait
    return self._trait(name, -2)
traits.trait_errors.DelegationError: The 'Value' attribute of a 'C2' object has a delegate which does not have traits.
test

Thanks

FedeMiorelli commented 4 years ago

Hi @corranwebster did you have a chance to check if the bug is reproducible on your side?

From further investigation, if I suppress the exception (with push_exception_handler to an empty function), the unpickling proceeds and the object is restored, however not all the delegation wiring is setup correctly as some behavior in the GUI ends up being broken.

Cheers, Federico

corranwebster commented 4 years ago

I think I missed your code from a couple of weeks ago. I can replicate the error, and this does look like a bug.