enum34, the Enum backport, specifically uses `protocol=HIGHEST_PROTOCOL`, while the current enum tests just use the default.

Running the enum34 test expose an issue with pickle protocol 4:

======================================================================
ERROR: test_subclasses_with_getnewargs (__main__.TestEnum)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_enum.py", line 1022, in test_subclasses_with_getnewargs
    self.assertEqual(loads(dumps(NI5, protocol=HIGHEST_PROTOCOL)), 5)
_pickle.PicklingError: Can't pickle <class '__main__.TestEnum.test_subclasses_with_getnewargs.<locals>.NamedInt'>: attribute lookup TestEnum.test_subclasses_with_getnewargs.<locals>.NamedInt on __main__ failed

======================================================================
ERROR: test_tuple_subclass (__main__.TestEnum)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_enum.py", line 1100, in test_tuple_subclass
    self.assertTrue(loads(dumps(SomeTuple.first, protocol=HIGHEST_PROTOCOL)) is SomeTuple.first)
_pickle.PicklingError: Can't pickle <enum 'SomeTuple'>: attribute lookup TestEnum.test_tuple_subclass.<locals>.SomeTuple on __main__ failed

Pickle tests should test all protocols in range(pickle.HIGHEST_PROTOCOL + 1).

Working on fixing tests now.  Not sure I can fix pickle (at least not in time for RC1).

Those tracebacks don't correspond to line numbers in the stdlib enum.py.

Regardless, the two tests fail understandably: they define a class in the local namespace and then monkeypatch globals() with it, but don't adjust the __qualname__.

Thanks, Antoine, that was what I needed.

Serhiy, the minimum supported pickle protocol is 2.  Now testing all protocols from 2 to HIGHEST_PROTOCOL, inclusive.

New changeset 35f57ab9389b by Ethan Furman in branch 'default':
Close issue20534: test_enum now tests all supported pickle protocols (2 - HIGHEST_PROTOCOL, inclusive).
http://hg.python.org/cpython/rev/35f57ab9389b

But why the minimum supported pickle protocol is 2? Here is a patch which makes enums pickleable with all protocols.

Because I didn't know how to make it work with 0 and 1.  Thank you!  I'll get that in today.

Okay, I went with __reduce__ since we don't ever use the pickle protocol information.

I added a test for class-nested Enums since protocol 4 supports it.

I left the test for test_subclasses_without_getnewargs alone as the point of that test is to make sure that _make_class_unpicklable is working properly, not to see if we can somehow get any of it to pickle.

Running all tests now.

I also removed the protocol 2 warning from the docs, and added this note:

.. note::

    With pickle protocol version 4 it is possible to easily pickle enums
    nested in other classes.

Pickle will prefer the __reduce_ex__() method over the __reduce__() method. If base class defined the __reduce_ex__() method, Enum.__reduce__() will be ignored.

> I left the test for test_subclasses_without_getnewargs alone as the point of that test is to make sure that _make_class_unpicklable is working properly, not to see if we can somehow get any of it to pickle.

Without these changes pickling failed, but not because _make_class_unpicklable.

I added other comments on Rietveld.

The version 2 docs:
http://docs.python.org/2/library/pickle.html#object.__reduce_ex__:
-------------------------------------------------------------------------
The object class implements both __reduce__() and __reduce_ex__();
however, if a subclass overrides __reduce__() but not __reduce_ex__(),
the __reduce_ex__() implementation detects this and calls __reduce__().
-------------------------------------------------------------------------

The current docs may say the same thing, but much less clearly:
http://docs.python.org/dev/library/pickle.html#object.__reduce_ex__:
-------------------------------------------------------------------------
Alternatively, a __reduce_ex__() method may be defined. The only
difference is this method should take a single integer argument, the
protocol version. When defined, pickle will prefer it over the
__reduce__() method. In addition, __reduce__() automatically becomes
a synonym for the extended version. The main use for this method is to
provide backwards-compatible reduce values for older Python releases.
-------------------------------------------------------------------------

Ethan commented:
----------------
> I left the test for test_subclasses_without_getnewargs alone as the point of
> that test is to make sure that _make_class_unpicklable is working properly,
> not to see if we can somehow get any of it to pickle.

Serhiy replied:
---------------
> Without these changes pickling failed, but not because _make_class_unpicklable.

Well, the direct cause is that the Enum class in question failed to define __getnewargs__; the indirect cause is that without __getnewargs__ the metaclass will call _make_class_unpicklable.

So neither the enum nor its members should pickle.

> -------------------------------------------------------------------------
> The object class implements both __reduce__() and __reduce_ex__();
> however, if a subclass overrides __reduce__() but not __reduce_ex__(),
> the __reduce_ex__() implementation detects this and calls __reduce__().
> -------------------------------------------------------------------------

This is about default  object.__reduce_ex__() implementation. But base class 
can override it.

>>> import enum, pickle
>>> class C(int):
...     def __reduce_ex__(self, proto):
...         return C, (int(self),)
... 
>>> class E(C):
...     X = C(42)
... 
>>> y = C(42)
>>> pickle.loads(pickle.dumps(y))
42
>>> type(pickle.loads(pickle.dumps(y)))
<class '__main__.C'>
>>> pickle.loads(pickle.dumps(E.X))
42
>>> type(pickle.loads(pickle.dumps(E.X)))
<class '__main__.C'>

Well, the direct cause is that the Enum class in question failed to define 
__getnewargs__; the indirect cause is that without __getnewargs__ the 
metaclass will call _make_class_unpicklable.

> Well, the direct cause is that the Enum class in question failed to define
> __getnewargs__; the indirect cause is that without __getnewargs__ the
> metaclass will call _make_class_unpicklable.
> 
> So neither the enum nor its members should pickle.

Actually they don't pickle due to wrong __module__. After adding

    NEI.__module__ = NamedInt.__module__ = __name__

PicklingError no longer raised for enum class.

And "BadPickle.__qualname__ = 'BadPickle'" in test_exploding_pickle is 
redundant, because BadPickle.__qualname__ already is 'BadPickle' (due to the 
module argument in constructor).

The only thing I hate more than being wrong is being wrong because Python isn't acting the way I think it should.  :/

So, __qualname__ is not set properly when using the function API (although it has nothing to do with manually setting __module__ (I removed it, tested, no difference)).

And, if a subclass defines __reduce__, but an ancestor class defines __reduce_ex__, the ancestor class wins -- this seems to completely defeat the purpose of subclassing.

I'll add a 'qualname' parameter (mirroring the 'module' parameter) to the function API, and I'll use __reduce_ex__.

A little experimenting shows that if the base class implements any of __reduce__, __reduce_ex__, __getnewargs__, or __getnewargs_ex__ that pickling will work, so modified that portion of Enum as well.

Serhiy commented:
-----------------
> Actually they don't pickle due to wrong __module__. After adding
> 
>    NEI.__module__ = NamedInt.__module__ = __name__
> 
> PicklingError no longer raised for enum class.

If you were to look at the `_make_class_unpicklable` function you would see it works by setting __module__ to <unknown>.  So, in fact, it does not pickle precisely because it has the wrong module, and your change undoes what Enum was tryng to do: if an instance cannot be pickled, don't let the class be pickled either.

I don't know if this was discussed, but as far as enums will be used for platform depending constants (as ENOENT or AF_UNIX), I think that enums should be pickled by name, not by value. Here is the __reduce_ex__ implementation:

    def __reduce_ex__(self, proto):
        if proto < 4:
            return getattr, (self.__class__, self._name_)
        return self.__class__.__qualname__ + '.' + self._name_

I do not recall if it was or not.  The main difference would be in how aliases were handled.  For example, if W and Z were the same value on system A, but different on system B, then going from A - B via pickle W and Z would still be the same using the current methods (pickling by value).

Considering that on system A, W and Z are two names for the exact same object, and that with SomeEnum(xx) the 'xx' is a value, how could we pickle just the name and be able to unpickle on sytem B?

New changeset 9f75f8a2cbb4 by Ethan Furman in branch 'default':
Close issue20534: all pickle protocols now supported.
http://hg.python.org/cpython/rev/9f75f8a2cbb4