[issue20653] Pickle enums by name
Roundup Robot added the comment: New changeset 010723a7bd25 by Ethan Furman in branch '3.4': Close issue20653: allow Enum subclasses to override __reduce_ex__ http://hg.python.org/cpython/rev/010723a7bd25 New changeset 737f2be5e80c by Ethan Furman in branch '3.4': Close issue20653: improve functional API docs; minor code changes http://hg.python.org/cpython/rev/737f2be5e80c New changeset 2c5a5fa0692c by Ethan Furman in branch '3.4': Issue20653: fix ReST for Enum http://hg.python.org/cpython/rev/2c5a5fa0692c -- ___ 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
[issue20653] Pickle enums by name
Roundup Robot added the comment: New changeset b637064cc696 by Ethan Furman in branch 'default': Close issue20653: improve functional API docs; minor code changes http://hg.python.org/cpython/rev/b637064cc696 -- resolution: - fixed stage: commit review - committed/rejected status: open - closed ___ 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
[issue20653] Pickle enums by name
Roundup Robot added the comment: New changeset 54ab95407288 by Ethan Furman in branch 'default': Issue20653: fix ReST for Enum http://hg.python.org/cpython/rev/54ab95407288 -- ___ 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
[issue20653] Pickle enums by name
Eli Bendersky added the comment: Can you upload the new patch? -- ___ 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
[issue20653] Pickle enums by name
Changes by Ethan Furman et...@stoneleaf.us: Added file: http://bugs.python.org/file34185/issue20653.stoneleaf.03.patch ___ 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
[issue20653] Pickle enums by name
Changes by Ethan Furman et...@stoneleaf.us: Added file: http://bugs.python.org/file34191/issue20653.stoneleaf.04.patch ___ 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
[issue20653] Pickle enums by name
Antoine Pitrou added the comment: __reduce_ex__ is not the preferred method: it's only necessary if you want so special-case according to the prototocol number. In most cases, this is not necessary so it is simpler to define __reduce__ and not __reduce_ex__. So I think the patch really should make use of __reduce__, not __reduce_ex__. -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Many comments, Eli's and Serhey's code changes incorporated. -- Added file: http://bugs.python.org/file34173/issue20653.stoneleaf.02.patch ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Antoine, If the mixed-in class defines __reduce_ex__, and the Enum class defines __reduce__, pickle will see that the Enum class has both, and will call the _ex__ method. It is, therefore, the preferred method (at least by pickle, which is what we are discussing). If you don't believe me, try it and see. -- ___ 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
[issue20653] Pickle enums by name
Antoine Pitrou added the comment: If the mixed-in class defines __reduce_ex__, and the Enum class defines __reduce__, pickle will see that the Enum class has both, and will call the _ex__ method. Ah, I understand your concern. You are using preferred in a different sense. The pickle docs don't mention __reduce_ex__ as being preferred, as in you should use this one, on the contrary. -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Yeah, I was confused by that when I first read it as well. The 2.7 docs are even worse in that regard (so there has been some progress :). -- ___ 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
[issue20653] Pickle enums by name
Antoine Pitrou added the comment: Yeah, I was confused by that when I first read it as well. The 2.7 docs are even worse in that regard (so there has been some progress :). The docs are actually correct. They may be confusing because the customization possibilities are more numerous than people may expect. Rewordings welcome. -- ___ 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
[issue20653] Pickle enums by name
Eli Bendersky added the comment: Many comments, Eli's and Serhey's code changes incorporated. Looks better, thanks. I left some comments in Rietveld. -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Antoine commented: -- The pickle docs don't mention __reduce_ex__ as being preferred, as in you should use this one, on the contrary. Are we reading the same docs? http://docs.python.org/dev/library/pickle.html#object.__reduce_ex__ --- [...] When defined, pickle will prefer it over the __reduce__() method. -- ___ 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
[issue20653] Pickle enums by name
Antoine Pitrou added the comment: Antoine commented: -- The pickle docs don't mention __reduce_ex__ as being preferred, as in you should use this one, on the contrary. Are we reading the same docs? Are we reading the same comments? -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: On 02/21/2014 11:26 AM, Antoine Pitrou wrote: Are we reading the same comments? LOL, apparently not. -- ___ 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
[issue20653] Pickle enums by name
Eli Bendersky added the comment: Ethan, the patch you committed here seems obscure to me. Why __reduce_ex__ and not __reduce__? Where are the accompanying documentation changes? Can you clarify more how the full logic of pickling now works - preferably in comments withing the code? -- versions: +Python 3.5 -Python 3.4 ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: When I implemented pickle support I did not have a complete understanding of the pickle protocols nor how to best use them. As a result, I picked __getnewargs__ and only supported protocols 2 and 3 (4 didn't exist yet). Serhiy came along and explained a bunch of it to me, so now I know that __reduce_ex__ is the best choice [1] as it supports all the protocol levels, and is always used. The patch removes __getnewargs__ and makes __reduce_ex__ The One Obvious Way, but if it does not go in to 3.4.0 then I won't be able to remove __getnewargs__ because of backwards compatibility. All the tests still pass, and the new test for subclassing to pickle by name passes. [1] pickle supports two low-level methods: __reduce__ and __reduce_ex__; __reduce_ex__ is more powerful and is the preferred method. If a mix-in class to Enum defines __reduce_ex__ and Enum only defines __reduce__, Enum's __reduce__ will not be called. -- priority: normal - high resolution: fixed - stage: committed/rejected - commit review status: closed - open versions: +Python 3.4 -Python 3.5 ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: More explanation: __getnewargs__ is not used by pickle protocol 0 and 1; to support those protocols we need __reduce_ex__. Since __reduce_ex__ works for 0, 1, 2, 3, 4, ... there's no reason to have both __reduce_ex__ *and* __getnewargs__. -- ___ 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
[issue20653] Pickle enums by name
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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Thanks for your comments, Eli, I'll work on getting better comments in the code. The qualname comment is partially related to the pickling changes as it's necessary for protocol 4 (I forgot to put that comment in on the previous pickling change that addressed that issue). I reopened because a new (or another) patch with better comments is called for; I tagged it 3.4 because I thought default was still pointing to the 3.4 series. To answer here your other questions (which I'll also put in comments in the code): 1. if custom pickling is desired __reduce_ex__ needs to be defined 2. if (1) is not done, then normal pickle behavior takes place: i. check if this is a mixed or pure Enum ii. if mixed, check that mixed-in type (aka member_type) has a pickle protocol in place, and if it doesn't, have the resulting enum class have __reduce_ex__ be the _break_on_call_reduce method. We do this because even though the class and member will pickle, the member will fail to unpickle. 3. as I said in an earlier message, but didn't make clear: __reduce_ex__ is the preferred method. This means that if a class has both __reduce__ and __reduce_ex__, __reduce__ will be ignored (at least by pickle -- I haven't researched copy, which I think also uses __reduce__ and/or __reduce_ex__). And of course, if an ancestor class has __reduce_ex__, and a subclass has __reduce__, the subclass really has both. No, __reduce__ is not checked for. -- ___ 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
[issue20653] Pickle enums by name
Guido van Rossum added the comment: For the record I'm against it, but I don't have time to explain until after 3.4 has been released. -- nosy: +gvanrossum ___ 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
[issue20653] Pickle enums by name
Serhiy Storchaka added the comment: This is sad. Because after a release, change it will be much harder. -- ___ 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
[issue20653] Pickle enums by name
Guido van Rossum added the comment: It should not be changed after the release either. On Tue, Feb 18, 2014 at 11:57 AM, Serhiy Storchaka rep...@bugs.python.orgwrote: Serhiy Storchaka added the comment: This is sad. Because after a release, change it will be much harder. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20653 ___ -- ___ 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
[issue20653] Pickle enums by name
Barry A. Warsaw added the comment: On Feb 18, 2014, at 07:57 PM, Serhiy Storchaka wrote: This is sad. Because after a release, change it will be much harder. OTOH, if default-pickling-by-name could be overridden, so can default-pickling-by-value. -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: And it is now possible to override and pickle by name if your custom subclass so chooses. -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Patch allows subclass to override __reduce_ex__, which is useful if a mixed-in type does not have proper pickle support. -- Added file: http://bugs.python.org/file34138/issue20653.stoneleaf.01.patch ___ 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
[issue20653] Pickle enums by name
Roundup Robot added the comment: New changeset 587fd4b91120 by Ethan Furman in branch 'default': Close issue20653: allow Enum subclasses to override __reduce_ex__ http://hg.python.org/cpython/rev/587fd4b91120 -- nosy: +python-dev resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: Proposal to switch to pickle by name rejected, but Enum now allows __reduce_ex__ to be overwridden in subclasses. -- priority: release blocker - normal ___ 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
[issue20653] Pickle enums by name
Serhiy Storchaka added the comment: Ha! You just has committed a patch which I write right now. Thank you Guido for your time machine. -- ___ 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
[issue20653] Pickle enums by name
Larry Hastings added the comment: If you want this in 3.4.0, please create a 3.4 cherry-pick issue for it. Personally I would feel a lot more confident in the change if there was any evidence of it being reviewed before it was checked in. All I see is a four-minute window between the patch being uploaded and it being checked in. -- ___ 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
[issue20653] Pickle enums by name
Barry A. Warsaw added the comment: On Feb 17, 2014, at 07:03 AM, Serhiy Storchaka wrote: Currently enums are pickled by values. It means that if the value of enum is platform depending, pickling one enum you can unpickle other enum on other platform. It's probably a good idea to pickle by name by default, in order to support the stdlib use case. Please just be sure to preserve the ability for user code to pickle by value via subclassing. -- ___ 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
[issue20653] Pickle enums by name
Serhiy Storchaka added the comment: To pickle by value, the subclass needs only restore current implementation of __reduce_ex__(): def __reduce_ex__(self, proto): return self.__class__, (self.value,) -- ___ 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
[issue20653] Pickle enums by name
Ethan Furman added the comment: I agree that pickling by name is the better solution. Serhiy, could you explain how the un-pickling works with protocol 4? -- assignee: - ethan.furman ___ 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
[issue20653] Pickle enums by name
Serhiy Storchaka added the comment: This is new feature of protocol 4 (PEP 3154, section 'Serializing more lookupable objects'). When __reduce_ex__() returns a string (instead of a tuple), this is interpreted as the name of a global, and qualnames with dots are now supported in protocol 4. -- ___ 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
[issue20653] Pickle enums by name
Serhiy Storchaka added the comment: May be a number of tests which test pickling subclasses with or without some special methods are not needed longer. Fell free to improve my patch if you want to make all cleanup changes in one commit. -- ___ 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
[issue20653] Pickle enums by name
New submission from Serhiy Storchaka: Currently enums are pickled by values. It means that if the value of enum is platform depending, pickling one enum you can unpickle other enum on other platform. Here is a patch which makes enum pickling by name. It also get rid of not needed __getnewargs__(). See also discussions in issue20534 and on Python-Dev: http://comments.gmane.org/gmane.comp.python.devel/145536. -- components: Library (Lib) files: enum_pickle_by_name.patch keywords: patch messages: 211394 nosy: alexandre.vassalotti, barry, eli.bendersky, ethan.furman, larry, pitrou, serhiy.storchaka priority: release blocker severity: normal stage: patch review status: open title: Pickle enums by name type: behavior versions: Python 3.4 Added file: http://bugs.python.org/file34113/enum_pickle_by_name.patch ___ 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
