Repository navigation
Pickling of classes with a metaclass and copy_reg #51938
Description
| BPO | 7689 |
|---|---|
| Nosy | @ncoghlan, @pitrou, @jackdied, @avassalotti, @merwok, @durban |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2011-10-04.07:40:54.659>
created_at = <Date 2010-01-13.09:32:13.536>
labels = ['extension-modules', 'type-bug', 'library']
title = 'Pickling of classes with a metaclass and copy_reg'
updated_at = <Date 2011-10-04.17:15:31.103>
user = 'https://bugs.python.org/craigcitro'bugs.python.org fields:
activity = <Date 2011-10-04.17:15:31.103>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2011-10-04.07:40:54.659>
closer = 'pitrou'
components = ['Extension Modules', 'Library (Lib)']
creation = <Date 2010-01-13.09:32:13.536>
creator = 'craigcitro'
dependencies = []
files = ['15853', '21101']
hgrepos = []
issue_num = 7689
keywords = ['patch', 'needs review']
message_count = 18.0
messages = ['97702', '107954', '107956', '112245', '112247', '130733', '144856', '144857', '144858', '144859', '144860', '144862', '144863', '144864', '144865', '144900', '144903', '144906']
nosy_count = 13.0
nosy_names = ['ncoghlan', 'pitrou', 'jackdied', 'alexandre.vassalotti', 'bsilverthorn', 'eric.araujo', 'daniel.urban', 'craigcitro', 'dalleyg', 'python-dev', 'fbissey', 'nlhepler', 'Brent.Payne']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue7689'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']
Activity
craigcitro commentedon Jan 13, 2010
Currently, it's impossible to use the usual pickle mechanisms to pickle a dynamically created class, even if the user requests a different pickling mechanism via copy_reg. The attached patch makes this customization possible by simply transposing two blocks of code.
Longer explanation: Classes are pickled by name, which is of course problematic when trying to pickle a dynamically created class. The natural solution would be to create a __reduce__ method on the metaclass. However, as mentioned in bpo-494904, this won't work -- the issue is that for any class C with a metaclass M, C.__reduce__ is the unbound method for instances of class C, as opposed to the bound method M.__reduce__ for C (i.e. viewing C as an instance of M). Guido's patch on that ticket does the sensible thing, which is force the class to be pickled by name -- this is fine, until you want to pickle a class that's created at runtime.
The copy_reg module exists to handle custom pickling of objects, which is exactly what's needed here. However, the code from bpo-494904 that checks for instances of a metaclass does this *just before* it looks at the copy_reg dispatch table. The patches just reorder these tests in pickle.py and cPickle.c, so that one can register a custom pickler for instances of a metaclass.
Comments: First, let me preemptively say that I do indeed have a use case for this fix. We ran into this bug working on Sage (http://www.sagemath.org), where we create lots of dynamic classes at runtime (to model mathematical relationships between different kinds of objects). There's a healthy mix of dynamic and non-dynamic classes floating around, and we want the user to be able to pickle them without having to know anything about how the class was created. We could create our own pickling function, but we'd much rather just use the default Python mechanisms, especially since it's such a small fix.
Second, this patch is fairly "safe," in the sense that it's pretty unlikely it can break any existing code. The only way it could break is if someone created a dynamic class, registered a pickler for it with copy_reg, and was depending on calls to pickle to fail.
Third, there's a few extra lines of code in the chunk the patch moves around coming from ticket bpo-502085. These are there to deal with versions of Boost circa 2002. Is it worth removing these?
The attached patch is against trunk (r77421). At least as I write this, the same patch should apply against the py3k branch (up to renaming copy_reg to copyreg, anyway), but I'm happy to do the legwork of rebasing it as needed.
Also, I'd happily review something else in exchange for a review of this. We'll be keeping this patch around in Sage until it gets merged, so the sooner the better.
Authors: Nicolas Thiery (nthiery@users.sf.net) first hunted this down and wrote a patch for cPickle.c. I did the (fairly trivial) work of mirroring the fix in pickle.py, and added the tests in pickletester.
dalleyg commentedon Jun 16, 2010
Another use case: for distributed processing, it's handy to be able to pickle interactive functions and functions that are part of a script. The user can then remotely execute a broader set of functions than can be pickled by default. This is especially helpful for interactive exploration of data.
Pickling most types of functions is possible by serializing a function's bytecode, etc. and registering a handler with copy_reg. Unfortunately, this handler is ignored under some conditions (e.g. copy_reg is ignored when attempting to serialize top-level functions in a script) but the copy_reg handler does get used for lambdas and inner functions. This patch looks like it will allow FunctionType's pickle handler to be overridden in all cases.
merwok commentedon Jun 16, 2010
Thanks for your report and patch. I’m editing the versions field: New features and bug fixes go to the active branch, py3k (future 3.2), while only security and documentation fixes are allowed to go in stable branches (2.6 and 3.1). Current trunk (2.7) is frozen, since it’s in release candidate stage; if your bug is fixed in py3k really soon, we’ll ask the release manager Benjamin Peterson if he agrees to let it into 2.7. Thanks again!
merwok commentedon Aug 1, 2010
I was mistaken: bug fixes go in stable releases too.
merwok commentedon Aug 1, 2010
Not an expert, but the Python parts of your patch look good to me.
durban commentedon Mar 13, 2011
Attaching an updated patch for py3k.
Note, that it is possible, that the copy module also should be fixed similarly (but currently that can't even copy non-dynamic classes with a metaclass, see bpo-11480).
nlhepler commentedon Oct 4, 2011
Hello all, sorry to be a bother, but what's the progress on this issue? I have a codebase that requires resolution of this issue to enable multiprocessing. What are the remaining outstanding problems herein preventing the attached patches from being merged?
pitrou commentedon Oct 4, 2011
+ def __eq__(self, other):
+ r = (type(self) == type(other))
+ if r:
+ return r
I think this should be "if not r".
craigcitro commentedon Oct 4, 2011
Antoine -- why do you want to switch "if r" for "if not r"?
If we did, the test would just confirm that the unpicked object was of the same type as the original; if we were going to do that, we might as well just replace the whole
__cmp__function with justreturn cmp(type(self), type(other)). On the flipside, I could see an argument for adding more content to the test, but that seemed like overkill.pitrou commentedon Oct 4, 2011
Craig: I'm talking about the __eq__ version (durban's patch). The __cmp__ version is probably fine.
craigcitro commentedon Oct 4, 2011
Antoine -- ah, that makes sense. Is that the only blocker? I've let this patch rot on the vine a long time; if so, I'll happily switch
__eq__back to__cmp__and re-post if it'll get submitted.pitrou commentedon Oct 4, 2011
No need, I'll do it myself.
python-dev commentedon Oct 4, 2011
New changeset 760ac320fa3d by Antoine Pitrou in branch '3.2':
Issue bpo-7689: Allow pickling of dynamically created classes when their
http://hg.python.org/cpython/rev/760ac320fa3d
New changeset 46c026a5ccb9 by Antoine Pitrou in branch 'default':
Issue bpo-7689: Allow pickling of dynamically created classes when their
http://hg.python.org/cpython/rev/46c026a5ccb9
python-dev commentedon Oct 4, 2011
New changeset 64053bd79590 by Antoine Pitrou in branch '2.7':
Issue bpo-7689: Allow pickling of dynamically created classes when their
http://hg.python.org/cpython/rev/64053bd79590
pitrou commentedon Oct 4, 2011
This is fixed now, thank you!
BrentPayne commentedon Oct 4, 2011
will the 2.7 patch also be incorporated into a 2.7 release?
pitrou commentedon Oct 4, 2011
Yes, it will.
ncoghlan commentedon Oct 4, 2011
Specifically, 2.7.3. A date for that has not yet been set, but somewhere in the December/January time frame is likely.