Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make OMD picklable under py3, fixes #337 #338

Merged
merged 2 commits into from
Apr 21, 2023
Merged

Make OMD picklable under py3, fixes #337 #338

merged 2 commits into from
Apr 21, 2023

Conversation

mahmoud
Copy link
Owner

@mahmoud mahmoud commented Apr 18, 2023

For some reason this test passed under py2, which I guess is telling as to how long it's been since I was actively pickling.

Copy link
Contributor

@allanlei allanlei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be additional pickling issues. The deserialized value loses the multi-value for getlist.

In [48]: value = OrderedMultiDict([('a', 1), ('a', 2), ('b', 3)])

In [49]: value.getlist('a')
Out[49]: [1, 2]


In [50]: pickle.loads(pickle.dumps(value)).getlist('a')
Out[50]: [2]

# `iteritems` seems to be correct
In [16]: list(value.iteritems(multi=True))
Out[16]: [('a', 1), ('a', 2), ('b', 3)]

In [11]: list(pickle.loads(pickle.dumps(value)).iteritems(multi=True))
Out[11]: [('a', 1), ('a', 2), ('b', 3)]

@mahmoud
Copy link
Owner Author

mahmoud commented Apr 21, 2023

Right you are. Odd that Py2 was so much better behaved than Py3.

Just a couple implementation notes:

  • The __getnewargs__ approach in the docs almost works, but because OMD inherits from dict, the builtin implementation overwrites the values via __setitem__ calls after __new__. That's why only individual values made it through; OMD's __setitem__ clears previously-added values.
  • __setstate__ apparently runs after the __setitem__ calls mentioned above, allowing us to reset the multi-value. This does mean that there's some extra work happening during unpickling. Maybe a fancy __reduce_ex__ could get around this, but the current approach seems fine for now.

@mahmoud mahmoud merged commit 0341ab3 into master Apr 21, 2023
@mahmoud mahmoud deleted the py3-picklable-omd branch October 13, 2023 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants