This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: deepcopy erroneously doesn't call __setstate__ if __getstate__ returns empty dict
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 2.7, Python 2.6
process
Status: closed Resolution: works for me
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: amaury.forgeotdarc, belopolsky, r.david.murray, serhiy.storchaka, shauncutts
Priority: normal Keywords: easy

Created on 2009-09-02 21:29 by shauncutts, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
deepcopy_bug.py shauncutts, 2009-09-02 21:29 Gives example showing bug
Messages (7)
msg92186 - (view) Author: Shaun Cutts (shauncutts) Date: 2009-09-02 21:29
Line 335 of copy.py guards call to __setstate__ with

if state:
    ...

However, __getstate__ may legitimately return an empty dictionary even
if __setstate__ still needs to be called. For example,
__setstate__/__getstate__ pair may not want to "persist" default values
(as is the case for me).

The fix would be to change this line to (e.g.):

if state is not None:
    ...
msg92200 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-09-03 11:30
This is precisely documented here:
http://docs.python.org/library/pickle.html#object.__setstate__
"Note: For new-style classes, if __getstate__() returns a false value,
the __setstate__() method will not be called."

If you want some default value even when the state is empty, you could
set it in the __new__ method:
[__new__ is always called, but __init__ is skipped by the copy protocol]

class A(object):
    def __new__(cls):
        self = super(cls, A).__new__(cls)
        self.a = 1
        return self
    def __setstate__(self, d):
        self.__dict__.update(d)
    def __getstate__(self):
        d = self.__dict__.copy()
        d.pop('a')
        return d

The __setstate__ is even not necessary here, since it implements the
default behaviour.
msg98837 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-02-04 16:21
The documentation also says, "if the object defines both a __getstate__ and a __setstate__ method, the state object does not need to be a dictionary and the methods can do what they want."  In issue 7848 (which I will close as a duplicate of this issue), the object wants to return a single integer as the state, and that integer can happen to be zero, so clearly "anything they want" is demonstrably not true.  Granted that this can also be worked around with a __new__ method, it feels like a broken API.  It unnecessarily complicates the implementation of objects that implement the protocol but whose state object can legitimately taken on a False value.  It also breaks the principle of least surprise for someone taking advantage of the "anything they want" clause (it may not occur to them that the 'black blox' state they are passing between their methods could take on a False value...and therefore break their code).

Since this was clearly a conscious choice for new-style classes, does anyone know why it was made?
msg98838 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-02-04 16:22
At the very least the documentation should be updated to warn that cooperating __getstate__ and __setstate__ methods must make sure the state object can never take on a False value.
msg108922 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-29 17:29
I see the following under <http://docs.python.org/dev/py3k/library/pickle.html#pickling-class-instances>:

Note If __getstate__() returns a false value, the __setstate__() method will not be called.  That was added in r62216 and highlighted in r67045.

Is there anything else that needs to be done here?
msg108943 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-06-29 20:02
Given that the API is what is, then it looks like the needed doc improvements have been done.
msg265089 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-05-07 19:13
The documentation of the pickle module doesn't match PEP 307 and the implementation. The copy module was changed in issue25718 to match the implementation of the pickle module. This fixed this issue.
History
Date User Action Args
2022-04-11 14:56:52adminsetgithub: 51076
2016-05-07 19:13:14serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg265089
2010-06-29 20:02:14r.david.murraysetstatus: pending -> closed

messages: + msg108943
stage: resolved
2010-06-29 17:29:36belopolskysetstatus: open -> pending

nosy: + belopolsky
messages: + msg108922

assignee: belopolsky
2010-02-04 16:23:52r.david.murraylinkissue7848 superseder
2010-02-04 16:22:50r.david.murraysetmessages: + msg98838
2010-02-04 16:21:40r.david.murraysetstatus: pending -> open
priority: normal

versions: + Python 2.6, Python 3.1, Python 2.7, Python 3.2, - Python 2.5
keywords: + easy
nosy: + r.david.murray

messages: + msg98837
2009-09-03 11:30:54amaury.forgeotdarcsetstatus: open -> pending

nosy: + amaury.forgeotdarc
messages: + msg92200

resolution: works for me
2009-09-02 21:29:51shauncuttscreate