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
Support pickling objects with __new__ with keyword arguments with protocol 2+ #68352
Comments
Pickling of objects of classes whose __new__ mandates the use of keyword-only arguments is supported with protocol 4 (using a new opcode NEWOBJ_EX). But it is possible to implement this feature with protocol 2+ (less efficiently than with NEWOBJ_EX). __new_ex__ is pickled as partial(cls.__new__, cls, *args, **kwargs). Pickled data is compatible with older Python releases up to 2.7 (bpo-5228). Proposed patch adds support of __new__ with keyword arguments with protocols 2 and 3. |
"Pickling of objects of classes whose __new__ mandates the use of keyword-only arguments is supported with protocol 4 (using a new opcode NEWOBJ_EX)." Hum, can you please write a short example of such class which can only be pickled by the protocol 4 currently? Just for my information. I understand that some objects cannot be serialized by pickle with protocol lower than 4, whereas your change makes possible to serialize them on Python 3, and it will be possible to deserialize them on Python 2 and Python 3. If I understood correctly, the change makes sense. I reviewed the patch, it looks good to me. |
For now there are no such classes in the stdlib. No one implements __getnewargs_ex__. But an alternative implementation of pickling for methodcaller could use it (I implemented methodcaller pickling in bpo-22955 in different way, via __reduce_ex__, but used the same trick for passing keyword arguments to constructor). Note that multiprocessing uses default protocol 3 (bpo-23403), and this is not configurable. |
New changeset bc5894a3a0e6 by Serhiy Storchaka in branch 'default': |
Thank you for your review Victor. |
Buildbots failed. ====================================================================== Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_descr.py", line 4745, in test_reduce
obj.__reduce_ex__(proto)
AssertionError: ValueError not raised |
New changeset df33dbbef7bb by Serhiy Storchaka in branch 'default': |
Thank you Victor. Tests are fixed. I think the documentation needs to be updated. Here is a patch for pickle documentation. |
"Thank you Victor. Tests are fixed." Not all of them. Failure on FreeBSD: (I don't know what is test_pyclbr!?) http://buildbot.python.org/all/builders/AMD64%20FreeBSD%2010.0%203.x/builds/3866/ ====================================================================== Traceback (most recent call last):
File "/usr/home/buildbot/python/3.x.koobs-freebsd10/build/Lib/test/test_pyclbr.py", line 159, in test_others
cm('pickle')
File "/usr/home/buildbot/python/3.x.koobs-freebsd10/build/Lib/test/test_pyclbr.py", line 86, in checkModule
self.assertIsInstance(py_item, (FunctionType, BuiltinFunctionType))
AssertionError: <class 'functools.partial'> is not an instance of (<class 'function'>, <class 'builtin_function_or_method'>) |
New changeset 288953a787ce by Victor Stinner in branch 'default': |
Another fix would be to accept functools.partial type, but it looks like the test wants to exclude symbols which comes from other modules. |
At least, test_pyclbr was fixed by my change. Most 3.x buildbots are green again. |
Thank you Victor for fixing test_pyclbr. test_pyclbr looks fragile and may be there are bugs in pyclbr itself. But this is different issue. Could you please look at proposed documentation changes? |
Serhiy Storchaka added the comment:
Sure, done. |
Documentation patch updated. |
pickle_new_ex_protocol_2_doc_2.patch looks good to me. |
New changeset de982d8b7b15 by Serhiy Storchaka in branch 'default': |
Thanks. |
New changeset 7adc1d24d05b by Victor Stinner in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: