Skip to content

io: improve bytes handling - #9059

Merged
JelleZijlstra merged 4 commits into
python:mainfrom
sobolevn:bytes-io
Nov 2, 2022
Merged

io: improve bytes handling#9059
JelleZijlstra merged 4 commits into
python:mainfrom
sobolevn:bytes-io

Conversation

@sobolevn

Copy link
Copy Markdown
Member

IncrementalNewlineDecoder.decode does not support str, but supports bytearray and memoryview:

>>>importcodecs, io>>>dec=codecs.getincrementaldecoder('utf8')()
>>>d=io.IncrementalNewlineDecoder(dec, translate=True)
>>>d.decode('str')
Traceback (mostrecentcalllast):
File"<stdin>", line1, in<module>File"/Users/sobolev/.pyenv/versions/3.10.0/lib/python3.10/codecs.py", line321, indecodedata=self.buffer+inputTypeError: can'tconcatstrtobytes

Buffers:

>>>d.decode(bytearray(b'\r'))
'\n'>>>d.decode(memoryview(b'\r'))
'\n'>>>d.decode(b'\r')
'\n'

@github-actions

This comment has been minimized.

@srittau

srittau commented Nov 1, 2022

Copy link
Copy Markdown
Collaborator

It isn't that easy, unfortunately:

Python 3.9.2 (default, Feb 28 2021, 17:03:44) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import io
>>> d = io.IncrementalNewlineDecoder(None, translate=True)
>>> d.decode('str')
'str'
>>> d.decode(b'bytes')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: decoder should return a string result, not 'bytes'

This is the implemenation: https://github.com/python/cpython/blob/934b25dcc492dcbca4da9d63d0d71dc940fc0375/Modules/_io/textio.c#L284

PyObject*_PyIncrementalNewlineDecoder_decode(PyObject*myself,
PyObject*input, intfinal)
{
PyObject*output;
Py_ssize_toutput_len;
nldecoder_object*self= (nldecoder_object*) myself;
if (self->decoder==NULL) {
PyErr_SetString(PyExc_ValueError,
"IncrementalNewlineDecoder.__init__ not called");
returnNULL;
}
/* decode input (with the eventual \r from a previous pass) */if (self->decoder!=Py_None) {
output=PyObject_CallMethodObjArgs(self->decoder,
&_Py_ID(decode), input, final ? Py_True : Py_False, NULL);
}
else {
output=input;
Py_INCREF(output);
}
if (check_decoded(output) <0)
returnNULL;
/* ... */

So, there are code paths, where onlystr is allowed, as check_decoded() explicity checks for a str.

@sobolevn

Copy link
Copy Markdown
MemberAuthor

Ok, let's go with an easy solution. I don't have any other ideas :(

@github-actions

This comment has been minimized.

@JelleZijlstra

Copy link
Copy Markdown
Member

I'm not sure this is quite right either. The argument gets passed to codecs.IncrementalDecoder.decode, which is an abstract method. I left it as taking bytes when I reviewed codecs, because that's what BufferedIncrementalDecoder initializes its buffer to. There may be decoders that accept ReadableBuffer, but I'm not sure it's required. If we make the change in this PR, we should also change codecs.

@sobolevn

Copy link
Copy Markdown
MemberAuthor

Yes, you are correct. Here's the full sequence:

(abstract)codecs.IncrementalDecoder -> (abstract)codecs.BufferedIncrementalDecoder -> encodings.*.IncrementalDecoder

Then io.IncrementalNewlineDecoder uses some of the concrete types from encodings.*.
Types in encodings.* already have ReadableBuffer set in this PR: #9043

So, I will create a new PR to fix codecs. types in a moment.

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@sobolevn

Copy link
Copy Markdown
MemberAuthor

There's an unrelated failure in the CI.

@JelleZijlstra
JelleZijlstra merged commit f972bdf into python:mainNov 2, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@sobolevn@srittau@JelleZijlstra