classification
Title: Support pickling objects with __new__ with keyword arguments with protocol 2+
Type: enhancement Stage: resolved
Components: Extension Modules, Interpreter Core, Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, alexandre.vassalotti, haypo, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-05-11 10:49 by serhiy.storchaka, last changed 2015-11-17 11:15 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
pickle_new_ex_protocol_2.patch serhiy.storchaka, 2015-05-11 10:49 review
pickle_new_ex_protocol_2_doc.patch serhiy.storchaka, 2015-10-11 15:01 review
pickle_new_ex_protocol_2_doc_2.patch serhiy.storchaka, 2015-10-13 18:04 review
Messages (19)
msg242890 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-11 10:49
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 (issue5228).

Proposed patch adds support of __new__ with keyword arguments with protocols 2 and 3.
msg251830 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-09-29 09:37
"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.
msg251857 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-29 13:30
> 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.

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 issue22955 in different way, via __reduce_ex__, but used the same trick for passing keyword arguments to constructor).

Note that multiprocessing uses default protocol 3 (issue23403), and this is not configurable.
msg252743 - (view) Author: Roundup Robot (python-dev) Date: 2015-10-10 19:43
New changeset bc5894a3a0e6 by Serhiy Storchaka in branch 'default':
Issue #24164: Objects that need calling ``__new__`` with keyword arguments,
https://hg.python.org/cpython/rev/bc5894a3a0e6
msg252744 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-10 19:44
Thank you for your review Victor.
msg252773 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-11 07:50
Buildbots failed.

http://buildbot.python.org/all/builders/AMD64%20Debian%20root%203.x/builds/2807/steps/test/logs/stdio

======================================================================
FAIL: test_reduce (test.test_descr.PicklingTests)
----------------------------------------------------------------------
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
msg252809 - (view) Author: Roundup Robot (python-dev) Date: 2015-10-11 14:55
New changeset df33dbbef7bb by Serhiy Storchaka in branch 'default':
Issue #24164: Fixed test_descr: __getnewargs_ex__ now is supported in protocols 2 and 3.
https://hg.python.org/cpython/rev/df33dbbef7bb
msg252811 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-11 15:01
Thank you Victor. Tests are fixed.

I think the documentation needs to be updated. Here is a patch for pickle documentation.
msg252854 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-12 12:00
"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/


======================================================================
FAIL: test_others (test.test_pyclbr.PyclbrTest)
----------------------------------------------------------------------
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'>)

----------------------------------------------------------------------
msg252855 - (view) Author: Roundup Robot (python-dev) Date: 2015-10-12 12:38
New changeset 288953a787ce by Victor Stinner in branch 'default':
Issue #24164: Fix test_pyclbr
https://hg.python.org/cpython/rev/288953a787ce
msg252856 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-12 12:40
> New changeset 288953a787ce by Victor Stinner in branch 'default':
> Issue #24164: Fix test_pyclbr

Another fix would be to accept functools.partial type, but it looks like the test wants to exclude symbols which comes from other modules.
msg252897 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-12 22:23
At least, test_pyclbr was fixed by my change. Most 3.x buildbots are green again.
msg252916 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-13 05:33
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?
msg252924 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-13 09:49
Serhiy Storchaka added the comment:
> Could you please look at proposed documentation changes?

Sure, done.
msg252945 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-13 18:04
Documentation patch updated.
msg252946 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-10-13 18:11
pickle_new_ex_protocol_2_doc_2.patch looks good to me.
msg252950 - (view) Author: Roundup Robot (python-dev) Date: 2015-10-13 18:27
New changeset de982d8b7b15 by Serhiy Storchaka in branch 'default':
Issue #24164: Document changes to __getnewargs__ and __getnewargs_ex__.
https://hg.python.org/cpython/rev/de982d8b7b15
msg252951 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-13 18:29
Thanks.
msg254793 - (view) Author: Roundup Robot (python-dev) Date: 2015-11-17 11:15
New changeset 7adc1d24d05b by Victor Stinner in branch 'default':
Closes #25645: Fix a reference leak introduced by change bc5894a3a0e6 of the
https://hg.python.org/cpython/rev/7adc1d24d05b
History
Date User Action Args
2015-11-17 11:15:19python-devsetmessages: + msg254793
2015-10-13 18:29:17serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg252951

stage: patch review -> resolved
2015-10-13 18:27:20python-devsetmessages: + msg252950
2015-10-13 18:11:44hayposetmessages: + msg252946
2015-10-13 18:04:31serhiy.storchakasetfiles: + pickle_new_ex_protocol_2_doc_2.patch

messages: + msg252945
2015-10-13 09:49:42hayposetmessages: + msg252924
2015-10-13 05:33:48serhiy.storchakasetmessages: + msg252916
2015-10-12 22:23:26hayposetmessages: + msg252897
2015-10-12 12:40:08hayposetmessages: + msg252856
2015-10-12 12:38:35python-devsetmessages: + msg252855
2015-10-12 12:00:31hayposetmessages: + msg252854
2015-10-11 15:01:36serhiy.storchakasetfiles: + pickle_new_ex_protocol_2_doc.patch

messages: + msg252811
stage: resolved -> patch review
2015-10-11 14:55:16python-devsetmessages: + msg252809
2015-10-11 07:54:00Arfreversetnosy: + Arfrever
2015-10-11 07:50:16hayposetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg252773
2015-10-10 19:44:08serhiy.storchakasetstatus: open -> closed
messages: + msg252744

assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2015-10-10 19:43:02python-devsetnosy: + python-dev
messages: + msg252743
2015-09-29 13:30:34serhiy.storchakasetmessages: + msg251857
2015-09-29 09:37:52hayposetnosy: + haypo
messages: + msg251830
2015-09-28 17:13:53serhiy.storchakasetversions: + Python 3.6, - Python 3.5
2015-05-11 10:49:50serhiy.storchakacreate