Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-36785: PEP 574 implementation #7076
Conversation
cef20d0
to
8943edf
|
||
if (self->readinto == NULL) { | ||
/* Call read() and memcpy */ | ||
/* XXX do we want this fallback? pickle.py doesn't have it */ |
pitrou
May 3, 2019
Author
Member
@ogrisel Do you think we need to support "file-like" objects without a readinto() method?
ogrisel
May 4, 2019
•
Contributor
I don't have a strong opinion. Are there any such file objects in the standard library? The file-like objects that acts as wrappers for (de)compression algorithms such as GzipFile
seem to support readinto
.
pitrou
May 4, 2019
Author
Member
Actually, the doc mentions that API contract for file
is only to have read
and readline
, so I'm not sure it's ok to remove the fallback...
ammaraskar
Feb 19, 2020
Member
Was the fallback ever added back? Looks like this came up as a real issue on https://bugs.python.org/issue39681
c56f2dd
to
745e7be
Rebased. |
Doc comments |
:class:`io.BytesIO` instance, or any other custom object that meets this | ||
interface. | ||
Arguments *file*, *protocol*, *fix_imports* and *buffer_callback* have | ||
the same meaning as in :class:`Pickler`. |
Thanks Terry! I've applied the suggested changes now. |
Here is my review, enjoy :-) I don't know well the pickle module, so I mostly checked the doc and basic stuff. By the way, I really like the overall change and the PEP: it's a great enhancement, thanks! |
|
||
:class:`PickleBuffer` objects can only be serialized using pickle | ||
protocol 5 or higher. They are eligible for | ||
:ref:`out-of-band serialization <pickle-oob>`. |
pitrou
May 19, 2019
Author
Member
It is referenced at the end of the "Out-of-band Buffers" section. Since it's quite specialized, I'm not sure it's useful to mention it also here.
Thanks for the review @vstinner. I think I've addressed your comments now. |
LGTM. I didn't see any major issue, but I didn't review carefully the C code. Nice PEP, nice implementation, thanks ;-) |
I'm gonna merge soon since no other review came and 3.8b1 is due next Friday. |
https://bugs.python.org/issue36785