Eli Bendersky added the comment:
If you were enlightened about how to use the pickle protocols, please explains
this better in the code. Currently the code says:
# check for a supported pickle protocols, and if not present sabotage
+ # pickling, since it won't work anyway.
+ # if new class implements its own __reduce_ex__, do not sabotage
+ if classdict.get('__reduce_ex__') is None:
+ if member_type is not object:
+ methods = ('__getnewargs_ex__', '__getnewargs__',
+ '__reduce_ex__', '__reduce__')
+ if not any(map(member_type.__dict__.get, methods)):
+ _make_class_unpicklable(enum_class)
The comments aren't very useful since they rephrase in different words what the
code does, rather than explaining *why* it's being done. Please provide a patch
with improved comments that make the following explicit:
1. What's expected of subclasses that want custom pickling.
2. What __new__ does if subclasses don't provide (1) - this "sabotaging" thing
and under what conditions - what is the significance of the "member type is not
object" test and the significance of the next test.
@@ -192,8 +194,9 @@ class EnumMeta(type):
(i.e. Color = Enum('Color', names='red green blue')).
When used for the functional API: `module`, if set, will be stored in
- the new class' __module__ attribute; `type`, if set, will be mixed in
- as the first base class.
+ the new class' __module__ attribute; `qualname`, if set, will be stored
+ in the new class' __qualname__ attribute; `type`, if set, will be mixed
+ in as the first base class.
Is it only me, or this comment change is unrelated to the pickling change?
Also, this lacks change in the documentation. Your patch makes it possible for
subclasses to provide custom pickling, overriding Enum's pickling. Some place
should say explicitly what's expected of such classes (e.g. you rely on
__reduce_ex__ taking precedence over __reduce__? what if subclasses rather
define __reduce__? is it disallowed?)
I'm not sure why you reopened this issue and retagged it to 3.4 when you've
already commiteed the change to default (which is 3.5 now?) and issue #20679
exists explicitly to discuss the cherrypick?
Nits:
if classdict.get('__reduce_ex__') is None:
can be replaced by:
if '__reduce_ex__' not in classdict:
And:
if not any(map(member_type.__dict__.get, methods)):
Would be more Pythonic as:
if not any(m in member_type.__dict__ for m in methods)
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue20653>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
