Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom __getstate__, __setstate__ for slotted classes (or improve docs)? #512

Closed
andhus opened this issue Mar 3, 2019 · 4 comments · Fixed by #642
Closed

Support custom __getstate__, __setstate__ for slotted classes (or improve docs)? #512

andhus opened this issue Mar 3, 2019 · 4 comments · Fixed by #642

Comments

@andhus
Copy link
Contributor

andhus commented Mar 3, 2019

The assertion fails in the following example, but is ok with slots=False:

import attr, pickle

@attr.s(slots=True)
class MyClass(object):
    a = attr.ib()
    not_picklable = attr.ib()

    def __getstate__(self):
        return self.a, "replacement"

    def __setstate__(self, state):
        self.a, self.not_picklable = state

mc = MyClass("a", "b")
mc_new = pickle.loads(pickle.dumps(mc))
assert mc_new.not_picklable == "replacement"

This is clearly because attrs auto creates these methods on the new slots-class: https:/python-attrs/attrs/blob/master/src/attr/_make.py#L601

Is there another prefered way to solve this, or would it be possible to support these methods also for slots-classes? If not, it would be good to clarify this in the documentation for slotted-classes, it says:

You can support protocol 0 and 1 by implementing __getstate__ and __setstate__ methods yourself. Those methods are created for frozen slotted classes because they won’t pickle otherwise. Think twice before using pickle though.

This is confusing to me, since implementing these methods have no effect for slotted classes?

Somewhat related:
#139
#475

@hynek
Copy link
Member

hynek commented Mar 3, 2019

You have opened this issue in the best possible moment because I'm prototyping a new API right now and this might go in. No promises about timeline and whether it'll be python2-compatible for now tho.

@andhus
Copy link
Contributor Author

andhus commented Mar 4, 2019

Glad to hear! In the meantime, I made this suggestion: #513. I think it makes sense. Maybe I'm missing something, but isn't the docs (referenced above) misleading/inconsistent with current behaviour?

@andhus
Copy link
Contributor Author

andhus commented Mar 4, 2019

Ok, read the docs more carefully and now understand that "You can support protocol 0 and 1 by implementing __getstate__ and __setstate__ methods yourself" refers to __slots__ classes in general but not to attr.s(slots=True) classes. Still don't see any downside from making it possible to define these methods in attr.s(slots=True) classes.

@hynek
Copy link
Member

hynek commented Mar 21, 2019

Ok, read the docs more carefully and now understand

With that you're way ahead of me because I don't understand that paragraph at all anymore. 😅

It would be great if we could rewrite it once this has been resolved.

hynek added a commit that referenced this issue Apr 28, 2020
Pass *add_getstate_setstate* to attr.s to control the generation.  Mostly
interesting for disabling it in slotted classes.

Fixes #512,#513
hynek added a commit that referenced this issue Apr 28, 2020
Pass *add_getstate_setstate* to attr.s to control the generation.  Mostly
interesting for disabling it in slotted classes.

Fixes #512,#513
hynek added a commit that referenced this issue Apr 28, 2020
Pass *getstate_setstate* to attr.s to control the generation.  Mostly
interesting for disabling it in slotted classes.

Fixes #512,#513
hynek added a commit that referenced this issue May 2, 2020
Pass *getstate_setstate* to attr.s to control the generation.  Mostly
interesting for disabling it in slotted classes.

Fixes #512,#513
hynek added a commit that referenced this issue May 11, 2020
* Add control to generate __[sg]etstate__

Pass *getstate_setstate* to attr.s to control the generation.  Mostly
interesting for disabling it in slotted classes.

Fixes #512,#513

* Update glossary.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants