Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions traits/has_traits.py
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,9 @@ def __getstate__(self):
for name in self.trait_names( type = 'delegate',
transient = False )
if name in dic ] ) )
# Add all instance traits
inst_traits = self._instance_traits()
result['__instance_traits__'] = inst_traits

# If this object implements ISerializable, make sure that all
# contained HasTraits objects in its persisted state also implement
Expand Down Expand Up @@ -1425,9 +1428,12 @@ def __setstate__ ( self, state, trait_change_notify = True ):
else:
# Otherwise, apply the Traits 3.0 restore logic:
self._init_trait_listeners()
inst_traits = state.pop('__instance_traits__', {})
self.trait_set( trait_change_notify = trait_change_notify, **state )
self._post_init_trait_listeners()
self.traits_init()
for attr, trait in inst_traits.iteritems():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cfarrow I notice that here we're calling add_trait after doing traits_init() and _post_init_trait_listeners(), but in the clone_traits block below, we're calling add_trait before those calls. Do you know whether there's a reason for the difference?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the reason. I'll look into the potential consequences. At the very least I'll make it consistent.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the doc string, traits_init should be called at the end of __setstate__. In practice, I don't think it matters. It is a no-op by default. The only place I have found it defined in the traits source is in HasUniqueStrings.

In any case, I moved it after all the add_traits to be consistent with the existing documentation and the behavior in clone_traits.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Chris. I also moved the _post_init_trait_listeners call, so that it's consistent with clone_traits.

self.add_trait(attr, trait)

self.traits_inited( True )

Expand Down Expand Up @@ -1779,6 +1785,9 @@ def clone_traits ( self, traits = None, memo = None, copy = None,
new = self.__new__( self.__class__ )
memo[ id( self ) ] = new
new._init_trait_listeners()
inst_traits = self._instance_traits()
for attr, trait in inst_traits.iteritems():
new.add_trait(attr, trait)
new.copy_traits( self, traits, memo, copy, **metadata )
new._post_init_trait_listeners()
new.traits_init()
Expand Down
17 changes: 17 additions & 0 deletions traits/tests/test_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys

from ..has_traits import HasTraits, Property, on_trait_change
from ..trait_errors import TraitError
from ..trait_types import Bool, DelegatesTo, Instance, Int, List
from ..testing.unittest_tools import unittest

Expand Down Expand Up @@ -189,6 +190,22 @@ def test_delegation_refleak(self):
# All the counts should be the same.
self.assertEqual(counts[warmup:-1], counts[warmup+1:])

def test_hastraits_pickle_deepcopy(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks as though this is a single test that tests both the pickling and the deepcopy behaviour in one. I think it should be split into two separate tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

from pickle import dumps, loads
from copy import deepcopy
a = HasTraits()
a.add_trait('foo', Int)
a.foo = 1
with self.assertRaises(TraitError):
a.foo = 'a'
pkld_a = dumps(a)
unpkld_a = loads(pkld_a)
with self.assertRaises(TraitError):
unpkld_a.foo = 'a'
copied_a = deepcopy(a)
with self.assertRaises(TraitError):
copied_a.foo = 'a'


if __name__ == '__main__':
unittest.main()