TextIOWrapper does not support reading bytearrays or memoryviews

[nikratio]

BPO	21057
Nosy	@loewis, @pitrou, @vstinner, @serhiy-storchaka
Files	issue21057.diff issue21057.diff issue_21057_r3.diff

^{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:

assignee = None
closed_at = <Date 2014-04-29.08:27:55.879>
created_at = <Date 2014-03-25.04:14:29.870>
labels = ['type-feature', 'library']
title = 'TextIOWrapper does not support reading bytearrays or memoryviews'
updated_at = <Date 2014-05-14.00:33:29.590>
user = 'https://bugs.python.org/nikratio'

bugs.python.org fields:

activity = <Date 2014-05-14.00:33:29.590>
actor = 'nikratio'
assignee = 'none'
closed = True
closed_date = <Date 2014-04-29.08:27:55.879>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2014-03-25.04:14:29.870>
creator = 'nikratio'
dependencies = []
files = ['34646', '34690', '35083']
hgrepos = []
issue_num = 21057
keywords = ['patch']
message_count = 20.0
messages = ['214776', '214777', '214787', '214852', '214853', '214871', '214874', '214886', '214891', '214893', '214899', '215015', '215293', '215985', '215987', '217405', '217490', '217491', '218480', '218495']
nosy_count = 6.0
nosy_names = ['loewis', 'pitrou', 'vstinner', 'nikratio', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue21057'
versions = ['Python 3.5']

[nikratio]

In Python 3.4, TextIOWrapper can not read from streams that return bytearrays or memoryviews:

from io import TextIOWrapper, BytesIO
class MyByteStream(BytesIO):
    def read1(self, len_):
        return memoryview(super().read(len_))
bs = MyByteStream(b'some data in ascii\n')
ss = TextIOWrapper(bs, encoding='ascii')
print(ss.read(200))

results in:

TypeError: underlying read1() should have returned a bytes object, not 'memoryview'

I don't think there's any good reason for TextIOWrapper not accepting any bytes-like object (especially now that b''.join finally accepts bytes-like objects as well).

[nikratio]

If someone is willing to review, I'd be happy to write a patch for this.

[vstinner]

I'm interested to review a patch, but I'm not sure that read1() can return a type different than bytes. You can use the Py_buffer API for your patch.

[serhiy-storchaka]

read1() should return bytes. MyByteStream doesn't implement the io.BufferedIOBase interface.

[nikratio]

On 03/25/2014 01:39 PM, Serhiy Storchaka wrote:

read1() should return bytes. MyByteStream doesn't implement the io.BufferedIOBase interface.

Indeed, this is what this issue is about :-).

The question is: is there a good reason to require io.BufferedIOBase
implementors to return bytes rather than any bytes-like object from read1?

I'd also argue that the current documentation of io.BufferedIOBase
actually does not clearly require read1 to return bytes. The exact
formulation is "Read and return up to *size* bytes" (note that "bytes"
is not interpreted text). This can just as easily read as "return binary
data of up to *size* bytes using one of the customary Python types".

So if it turns out that there is a good reason for this requirement, the
documentation should at least make this requirement more explicit.

Best,
-Nikolaus

--
Encrypted emails preferred.
PGP fingerprint: 5B93 61F8 4EA2 E279 ABF6 02CF A9AD B7F8 AE4E 425C

         »Time flies like an arrow, fruit flies like a Banana.«

[pitrou]

bytes objects have two things going for them:

• they have the full bytes API (all the startswith(), etc. methods) - not only buffer access
• they are immutable: you can keep an internal reference to a bytes object and be sure it won't change under your feet

This is especially handy when writing C code.

[nikratio]

Yes, bytes objects have some advantages. But if a bytes object is desired, it can always be created from bytes-like object. If a BufferedIOBase instance is required to only provide bytes objects, this conversion is forced even when it may not be necessary.

If someone is willing to do the work (and I am), is there a reason *not* to allow TextIOWrapper to accept bytes-like objects?

[serhiy-storchaka]

If someone is willing to do the work (and I am), is there a reason *not* to
allow TextIOWrapper to accept bytes-like objects?

Yes, there are. The code which works only with bytes is much simpler. Not only
TextIOWrapper, but many other classes in the stdlib (GzipFile, BZ2File,
LZMAFile, ZipFile) expect that the read method of underlied file object returns
bytes, so your MyByteStream is just broken for such cases.

[vstinner]

2014-03-26 0:35 GMT+01:00 Antoine Pitrou <report@bugs.python.org>:

• they are immutable: you can keep an internal reference to a bytes object and be sure it won't change under your feet

Is it possible to request this feature using PyObject_GetBuffer()? I
don't see such flag.

If it's not possible, a new flag is maybe needed?

[vstinner]

class MyByteStream(BytesIO):
def read1(self, len_):
return memoryview(super().read(len_))
bs = MyByteStream(b'some data in ascii\n')

I guess that you are trying to implement a zero-copy I/O. The problem is that BytesIO does copy data. Example:

>>> data=b'abcdef'
>>> x=io.BytesIO(data)
>>> x.read() is  data
False

Before trying to avoid copies in the "buffered" layer, something should be done for the "raw" layer (BytesIO in this case).

[nikratio]

On 03/26/2014 03:43 AM, STINNER Victor wrote:

> class MyByteStream(BytesIO):
> def read1(self, len_):
> return memoryview(super().read(len_))
> bs = MyByteStream(b'some data in ascii\n')

I guess that you are trying to implement a zero-copy I/O. The problem is that BytesIO does copy data.

Right on the first count, but wrong on the second. The class I'm
concerned with wants to do zero-copy I/O, but is not related to BytesIO.
I only picked that to produce a minimal example.

[nikratio]

I'm attaching a patch that enables TextIOWrapper to work with bytes-like objects from the underlying file descriptor.

The code changes are pretty small, without introducing any significant additional complexity.

For streams providing bytes objects, this patch does not change anything. For streams that are able to provide bytearrays or memoryviews, this patch removes the overhead that would be incurred by explicitly converting to bytes.

[nikratio]

Thanks for the feedback! I have attached an updated patch.

I did not include any testcase because the patch did not create any new code paths, so I was assuming it'd be covered by the existing test case. But of course I was wrong. In the revised patch, I added a testcase based on your example of a more complex memoryview.

(Note, however, that even with the previous implementation using nbytes = PySequence_Size(input_chunk) this test does not fail, because nbytes is used only to estimate the size of the text string).