classification
Title: Python and C implementations of io are out of sync
Type: behavior Stage: resolved
Components: IO Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, benjamin.peterson, gregory.p.smith, jcon, laura, pitrou, python-dev, rbcollins, stutzbach
Priority: normal Keywords: easy, patch

Created on 2010-09-15 08:53 by pitrou, last changed 2015-05-20 19:52 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
missing_methods.py stutzbach, 2010-09-15 13:36
issue9858.patch laura, 2015-04-27 06:14 review
Messages (21)
msg116440 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-15 08:53
RawIOBase is defined as having the readinto() method but it doesn't. It should at least have a failing one (raising io.UnsupportedOperation like the BufferedIOBase.{read,readinto} methods do).

This is, of course, mainly for auto-documenting purposes (with help() and friends).
msg116442 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-09-15 12:28
+1 for a failing one. (Does the base implementation not raise?)

Is it even possible to implement a real one without buffering?
msg116443 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-15 12:39
> +1 for a failing one. (Does the base implementation not raise?)

The base implementation doesn't exist :)

> Is it even possible to implement a real one without buffering?

FileIO does. It's the same as read(), except that you read into an existing buffer rather than allocating a new bytes object.
msg116444 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-09-15 13:36
Attached is a script to find all of the mismatches between the C and Python implementations.  There are several.  Below is the output:

RawIOBase C is missing: ['readinto', 'write']
StringIO C is missing: ['name']
StringIO python is missing: ['__getstate__', '__setstate__']
BufferedRandom python is missing: ['raw']
BlockingIOError python is missing: ['characters_written']
TextIOWrapper python is missing: ['buffer']
BufferedReader python is missing: ['raw']
BufferedWriter python is missing: ['raw']
BytesIO python is missing: ['__setstate__']

Since the Python version was the original reference implementation, adding the attributes missing from the C side seems to be a straightforward decision (and it should simply match whatever the Python version does).  It looks like a number of new attributes have creeped into the C side, which will require more thoughtful decision making.

We should probably double-check that each of these is documented, while we're at it.
msg116446 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-09-15 13:47
FWIW, I just opened Issue9859: Add tests to verify API match of modules with 2 implementations.
msg116455 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-09-15 15:06
2010/9/15 Daniel Stutzbach <report@bugs.python.org>:
>
> Daniel Stutzbach <daniel@stutzbachenterprises.com> added the comment:
>
> Attached is a script to find all of the mismatches between the C and Python implementations.  There are several.  Below is the output:
>
> BufferedRandom python is missing: ['raw']
> TextIOWrapper python is missing: ['buffer']
> BufferedReader python is missing: ['raw']
> BufferedWriter python is missing: ['raw']

These attributes exist; they're just not properties.
msg116460 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-09-15 15:44
> These attributes exist; they're just not properties.

Yes, I see.  They're added to the instance in the constructor, so they don't exist as attributes of the class.  Also in that category:

BlockingIOError python is missing: ['characters_written']

That leaves:

RawIOBase C is missing: ['readinto', 'write']
StringIO C is missing: ['name']
StringIO python is missing: ['__getstate__', '__setstate__']
BytesIO python is missing: ['__setstate__']

The Python version of StringIO throws an AttributeException on the .name attribute.  It's a property inherited from the TextIOWrapper.  Effectively, TextIOWrapper provides the .name attribute if the object that it's wrapping provides the .name attribute.  This behavior is undocumented.

Is that reasonable behavior?  Or should TextIOWrapper define .name always and return some suitable value if the wrapped object doesn't define .name? (e.g., None)

In any case, it needs documentation.
msg116462 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-09-15 15:51
2010/9/15 Daniel Stutzbach <report@bugs.python.org>:
>
> Daniel Stutzbach <daniel@stutzbachenterprises.com> added the comment:
>
>> These attributes exist; they're just not properties.
>
> Yes, I see.  They're added to the instance in the constructor, so they don't exist as attributes of the class.  Also in that category:
>
> BlockingIOError python is missing: ['characters_written']
>
> That leaves:
>
> RawIOBase C is missing: ['readinto', 'write']
> StringIO C is missing: ['name']
> StringIO python is missing: ['__getstate__', '__setstate__']
> BytesIO python is missing: ['__setstate__']

I'm not sure if this pickling stuff matters as long as they both pickle.

>
> The Python version of StringIO throws an AttributeException on the .name attribute.  It's a property inherited from the TextIOWrapper.  Effectively, TextIOWrapper provides the .name attribute if the object that it's wrapping provides the .name attribute.  This behavior is undocumented.
>
> Is that reasonable behavior?  Or should TextIOWrapper define .name always and return some suitable value if the wrapped object doesn't define .name? (e.g., None)

Yes, all the code expects an AttributeError on name is it's not
present. For example, FileIO only sets its name attribute if it is
passed a filename and not an fd.
msg116463 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-09-15 16:01
On Wed, Sep 15, 2010 at 10:51 AM, Benjamin Peterson
<report@bugs.python.org>wrote:

> I'm not sure if this pickling stuff matters as long as they both pickle.
>

I'm not sure either.  Do other Python implementations try to maintain a
compatible pickle format with CPython?  If so, I think CPython's C and
Python versions should also pickle to the same format.

> Yes, all the code expects an AttributeError on name is it's not
> present. For example, FileIO only sets its name attribute if it is
> passed a filename and not an fd.
>

Actually, FileIO sets the name attribute to the file descriptor if it isn't
passed a name:

1
msg116464 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-09-15 16:10
Roundup does not play well with Gmail when Gmail is in Rich Format mode ;-)

That example should have read:

>>> f = io.FileIO(1)
>>> f.name
1
msg116468 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-09-15 16:47
2010/9/15 Daniel Stutzbach <report@bugs.python.org>:
>
> Daniel Stutzbach <daniel@stutzbachenterprises.com> added the comment:
>
> Roundup does not play well with Gmail when Gmail is in Rich Format mode ;-)
>
> That example should have read:
>
>>>> f = io.FileIO(1)
>>>> f.name
> 1

Hmm, none the less other code expects it.
msg116501 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-09-16 00:39
> Hmm, none the less other code expects it.

Does that imply that there's some code that will break on FileIO objects that are created using a file descriptor instead of a filename?  If so, where?
msg116502 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-09-16 00:53
2010/9/15 Daniel Stutzbach <report@bugs.python.org>:
>
> Daniel Stutzbach <daniel@stutzbachenterprises.com> added the comment:
>
>> Hmm, none the less other code expects it.
>
> Does that imply that there's some code that will break on FileIO objects that are created using a file descriptor instead of a filename?  If so, where?

No, just that all code that access name expects an AttributeError,
indicating that when there is no "name" an error should result.
msg221727 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-27 20:50
Is there anything left to do on this or can it be closed as fixed?
msg241007 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-14 20:06
A test has been added as part of issue9859, it is marked with @unittest.skip as the API surface does not yet match.
msg242101 - (view) Author: Laura Rupprecht (laura) * Date: 2015-04-27 06:14
There were originally three methods present in RawIOBase that were not present in PyRawIOBase_Type:

1. readinto
2. write
3. __weakref__

I've created a patch that adds the first two to PyRawIOBase_Type. The python class readinto and write methods raise UnsupportedOperation, so the c methods return a PyExc_NotImplementedError.

The next major question I have is whether we need to implement a __weakref__ method or this should be ignored in the test.
msg243686 - (view) Author: Laura Rupprecht (laura) * Date: 2015-05-20 18:47
Can anyone provide feedback on this patch?
msg243688 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-05-20 19:08
I think ignoring weakref is wrong: it means the two implementations are different.
msg243689 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-05-20 19:41
__weakref__ is just an implementation detail of how heap types expose weak references (actually, I'm not sure why it's exposed at all).

Laura, thank you for contributing. Your patch looks good to me.
msg243691 - (view) Author: Roundup Robot (python-dev) Date: 2015-05-20 19:52
New changeset 962b42d67b9e by Antoine Pitrou in branch 'default':
Issue #9858: Add missing method stubs to _io.RawIOBase.  Patch by Laura Rupprecht.
https://hg.python.org/cpython/rev/962b42d67b9e
msg243692 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-05-20 19:52
I've pushed this to the default branch. Thanks!
History
Date User Action Args
2015-05-20 19:52:57pitrousetstatus: open -> closed
versions: - Python 2.7, Python 3.4
messages: + msg243692

resolution: fixed
stage: commit review -> resolved
2015-05-20 19:52:31python-devsetnosy: + python-dev
messages: + msg243691
2015-05-20 19:41:42pitrousetassignee: gregory.p.smith ->
resolution: accepted -> (no value)
messages: + msg243689
stage: needs patch -> commit review
2015-05-20 19:08:43rbcollinssetnosy: + rbcollins
messages: + msg243688
2015-05-20 18:47:17laurasetmessages: + msg243686
2015-04-27 06:14:47laurasetfiles: + issue9858.patch

nosy: + laura
messages: + msg242101

keywords: + patch
2015-04-14 23:04:06gregory.p.smithsetassignee: gregory.p.smith
2015-04-14 20:06:37gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg241007
2014-06-27 20:50:57BreamoreBoysetnosy: + BreamoreBoy

messages: + msg221727
versions: + Python 3.4, Python 3.5, - Python 3.1, Python 3.2
2011-06-02 01:13:10jconsetnosy: + jcon
2010-09-16 00:53:36benjamin.petersonsetmessages: + msg116502
2010-09-16 00:39:52stutzbachsetmessages: + msg116501
2010-09-15 16:47:51benjamin.petersonsetmessages: + msg116468
2010-09-15 16:10:32stutzbachsetmessages: + msg116464
2010-09-15 16:10:09stutzbachsetfiles: - unnamed
2010-09-15 16:01:35stutzbachsetfiles: + unnamed

messages: + msg116463
2010-09-15 15:51:09benjamin.petersonsetmessages: + msg116462
2010-09-15 15:44:16stutzbachsetmessages: + msg116460
2010-09-15 15:06:09benjamin.petersonsetmessages: + msg116455
2010-09-15 13:47:18stutzbachsetmessages: + msg116446
2010-09-15 13:36:53stutzbachsetfiles: + missing_methods.py
priority: low -> normal
title: RawIOBase doesn't define readinto -> Python and C implementations of io are out of sync
messages: + msg116444

resolution: accepted
2010-09-15 12:39:57pitrousetmessages: + msg116443
2010-09-15 12:28:24benjamin.petersonsetmessages: + msg116442
2010-09-15 08:54:28pitrousetnosy: + benjamin.peterson, stutzbach
2010-09-15 08:53:09pitroucreate