enthought / traits

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

Fix cloning issue with container traits #1624

Closed mdickinson closed 2 years ago

mdickinson commented 2 years ago

This PR makes a shallow fix for problems with clone_traits applied to List, Dict and Set traits. It doesn't try to touch deeper issues of disconnection of TraitsListObject, TraitDictObject and friends from their owning HasTraits object.

Closes #1622. The cause of that issue is that we were using a function lambda: None for object where a HasTraits object was expected. Inside TraitListObject we then take a weakref to that function. In most cases, the lambda function has no other references to it, so it's garbage collected immediately and when the weakref is dereferenced, it returns None. But in the deepcopy case the weakref target is kept alive for long enough that we try to use the actual lambda: None function as a HasTraits object. The solution is to allow and special-case an object of None in the TraitListObject constructor.

mdickinson commented 2 years ago

@sallenEnth Interested in reviewing? (Feel free to unassign yourself from review if not.)

mdickinson commented 2 years ago

That's unfortunately the part that's not fixed by this PR, and that's much harder to fix in general.

Apologies - looks like I'm wrong about this. So let's keep that test (thank you!), and I need to figure out how this is working when I expected it to fail ...

mdickinson commented 2 years ago

I need to figure out how this is working when I expected it to fail ...

Got it: the resulting value from applying deepcopy to the original TraitListObject is indeed disconnected from the object, but when we assign that copied value back to the object using setattr (here), we create a new TraitListObject that's connected to the newly cloned HasTraits object.