classification
Title: Recursive OrderedDict pickling
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, pitrou, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-05-03 19:40 by serhiy.storchaka, last changed 2016-06-12 08:27 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
OrderedDict_pickle_recursive.patch serhiy.storchaka, 2013-05-03 19:40
test_yaml.py rhettinger, 2014-11-02 08:41 Show the YAML breakage.
OrderedDict_pickle_PyYAML.patch serhiy.storchaka, 2014-11-02 14:56 review
Messages (11)
msg188325 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-05-03 19:40
Here is a patch which allows pickling of recursive OrderedDicts. Actually it simplifies __reduce__() code.
msg188344 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-05-04 08:08
In additional the patch decreases the size of the pickled data, speeds up pickling and unpickling.

$ ./python -c "import collections, pickle; od = collections.OrderedDict((i, i) for i in range(100000)); print(len(pickle.dumps(od)))"

Without patch: 1536827.
With patch: 737578.

$ ./python -m timeit -s "from pickle import dumps; import collections; od = collections.OrderedDict((i, i) for i in range(100000))"  "dumps(od)"

Without patch: 454 msec.
With patch: 361 msec.

$ ./python -m timeit -s "from pickle import dumps, loads; import collections; od = collections.OrderedDict((i, i) for i in range(100000)); x = dumps(od)"  "loads(x)"

Without patch: 1.41 sec.
With patch: 1.15 sec.
msg189662 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-05-20 13:50
Ping.
msg189688 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2013-05-20 20:22
The patch looks good to me. So go ahead and submit it. :)
msg189737 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-21 09:49
New changeset 56f25569ba86 by Serhiy Storchaka in branch 'default':
Issue #17900: Allowed pickling of recursive OrderedDicts.  Decreased pickled
http://hg.python.org/cpython/rev/56f25569ba86
msg230482 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-11-02 08:41
This patch caused a regression by breaking designed-in support for pickling using PyYAML (note, there was a test for yaml support, but this patch defeated the test).  As a result of the patch, ordering now gets lost during a dump/load roundtrip.

This change should not have been committed without my review (I would have caught the problem instantly).
msg230484 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-11-02 12:09
Can you explain what is broken independently of PyYAML?
msg230487 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-11-02 14:56
Oh, sorry, I did not notice test_yaml_linkage. And in any case this test was broken (should be rv[1][0] instead of rv[1]).

I don't like to revert these changes, and the reversion can break a code written for 3.4 which pickles recursive OrderedDicts. I suggest following solution. PyYAML calls __reduce_ex_(2) or __reduce__(). Therefore we can return to old behavior with protocol < 3 and left current behavior with protocol >= 3. Recursive OrderedDicts will be pickleable with default protocol and with highest protocol, and PyYAML will be happy.

Does this patch look good to your?
msg267778 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-08 04:57
I think it is too late to revert this change. Is it worth to add a workaround for PyYAML if it still hasn't fixed serialization of OrderedDicts?
msg268314 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-06-12 02:36
At this point, we can drop it.  PyYAML will just have to deal with it.
msg268347 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-12 08:27
Reported PyYAML issue: https://bitbucket.org/xi/pyyaml/issues/61/ordereddict-doesnt-round-trip.
History
Date User Action Args
2016-06-12 08:27:22serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg268347

stage: patch review -> resolved
2016-06-12 02:36:29rhettingersetassignee: rhettinger ->
messages: + msg268314
2016-06-08 04:57:02serhiy.storchakasetmessages: + msg267778
2015-07-21 07:45:15ethan.furmansetnosy: - ethan.furman
2014-11-07 17:17:33ethan.furmansetnosy: + ethan.furman
2014-11-02 19:30:16rhettingersetassignee: serhiy.storchaka -> rhettinger
2014-11-02 14:56:24serhiy.storchakasetfiles: + OrderedDict_pickle_PyYAML.patch

messages: + msg230487
stage: needs patch -> patch review
2014-11-02 12:09:01pitrousetmessages: + msg230484
2014-11-02 08:41:26rhettingersetstatus: closed -> open
files: + test_yaml.py
type: enhancement -> behavior

versions: + Python 3.5
messages: + msg230482
resolution: fixed -> (no value)
stage: resolved -> needs patch
2013-05-21 09:57:33serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2013-05-21 09:49:23python-devsetnosy: + python-dev
messages: + msg189737
2013-05-20 20:35:59serhiy.storchakasetassignee: serhiy.storchaka
2013-05-20 20:22:34alexandre.vassalottisetmessages: + msg189688
2013-05-20 13:50:55serhiy.storchakasetmessages: + msg189662
2013-05-04 08:08:18serhiy.storchakasetmessages: + msg188344
2013-05-03 19:40:16serhiy.storchakacreate