[issue20653] Pickle enums by name

[Eli Bendersky]

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 <report at bugs.python.org>
<http://bugs.python.org/issue20653>
_______________________________________