Skip to content
This repository has been archived by the owner on Feb 2, 2019. It is now read-only.

Allow AttrDict to be pickled #17

Merged
merged 1 commit into from
Nov 27, 2014
Merged

Conversation

jtratner
Copy link
Contributor

Previously, pickling would fail out with a recursion error (I believe because the code always assumed that certain properties were present and, on setting __dict__ in the unpickle, that caused an infinite loop). This defines a custom pickle to make things work as you'd expect. Since pickling could not work previously, this isn't a backwards-incompatible change.

Please tell me if you want me to change any formatting here or there or if you want any other changes for coding standards.

(previously would fail out with a recursion error)
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d868614 on jtratner:allow-attrdict-pickles into 00cbb69 on bcj:master.

@bcj
Copy link
Owner

bcj commented Nov 26, 2014

Thanks for adding this. At first glance, everything looks great. I'm going to look into the 3.3 test failure tonight, I'll let you know if I need any changed prior to merging this.

@jtratner
Copy link
Contributor Author

I was very surprised by that 3.3 error. Never seen a pickle error like
that. I can check it out too.

On Wed, Nov 26, 2014, 8:45 AM Brendan Curran-Johnson <
notifications@github.com> wrote:

Thanks for adding this. At first glance, everything looks great. I'm going
to look into the 3.3 test failure tonight, I'll let you know if I need any
changed prior to merging this.


Reply to this email directly or view it on GitHub
#17 (comment).

@bcj
Copy link
Owner

bcj commented Nov 27, 2014

I have found the bug, and arguably it is in my code (and arguably CPython 3.3 as well), not your code. The fix is simple, so I'll just add it prior to pushing to PyPI.

In case you care about the minutia:

I had previously run into this problem when dealing with copy.copy and copy.deepcopy, but found a fix before I actually worked out what was going on.

Pickle works by using the magic method __reduce__ (which seems to be reduce_2 in typeObject.c). __reduce__ looks for a method named __getnewargs__ which returns a tuple of arguments for __new__ (as there may be arguments that need to be set before __init__, or memory that a C object which would need to requests at __new__). If __getnewargs__ doesn't exist, it defaults to ().

Under Python 3.4, __reduce__ calls (the C equivalent of) getattr on the type. AttrDict doesn't implement __getnewargs__, so it safely defaults to ().

Under Python 3.3, the attribute is looked for on the instance. If default_factory is used, then instead of not returning the attribute, __getattr__ would call __missing__, which would return the result of the default_factory call (in the case of your test code, 1).

My code is at fault, because __getattr__ should only call __missing__ for public attributes, not any missing attributes.

The change (https://hg.python.org/cpython/rev/b328f8ccbccf) that caused this to work under 3.4 is referred to as a regression, so arguably 3.3's behavior is wrong (also, maybe some versions of 3.4, this change is from after 3.4's release). 2.7 uses a completely different method for attribute checks, but I'm guessing it must work on type not class.

Regardless, thanks for the pickle support, and for helping me find a bug in AttrDict.

bcj added a commit that referenced this pull request Nov 27, 2014
@bcj bcj merged commit e887e78 into bcj:master Nov 27, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants