Skip to content

bpo-29741: make some BytesIO methods accept integer types #560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zooba merged 9 commits into from
Aug 24, 2017
Merged

bpo-29741: make some BytesIO methods accept integer types #560

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
08e40b2
patch init commit
orenmn Mar 7, 2017
1ab4836
changed arg name of _io_StringIO_readline_impl
orenmn Mar 8, 2017
4beafeb
commit changes from PR #650 and #606
orenmn Mar 13, 2017
50d521d
merged master
orenmn Mar 13, 2017
fca4a4c
merged master.
orenmn Aug 6, 2017
f52a7bd
merge master
orenmn Aug 22, 2017
ccb19ae
updates according to zooba's review
orenmn Aug 22, 2017
7a5366d
add NEWS.d item
orenmn Aug 22, 2017
3e831f7
Remove name from NEWS item.
zooba Aug 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
patch init commit
  • Loading branch information
@orenmn
orenmn committed Mar 7, 2017
commit 08e40b2e82952d36caf5a83f4ae03430f9acf176
32 changes: 22 additions & 10 deletions Lib/_pyio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,11 @@ def nreadahead():
return 1
if size is None:
size = -1
elif not isinstance(size, int):
raise TypeError("size must be an integer")
else:
try:
size = size.__index__()
except AttributeError as err:
raise TypeError("an integer is required") from err

@zooba zooba May 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how size gets used after this, there shouldn't be any need to reassign it by calling __index__. Similarly for the other changes.

@orenmn orenmn Aug 6, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I remove the reassignment, I get TypeError: '<' not supported between instances of 'int' and 'IntLike' somewhere later, when the size var is compared to an int.
Did I misunderstand your comment?

@orenmn orenmn Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Steve?

@zooba zooba Aug 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - GitHub messages go to my work email, which means I don't see them unless I'm sneakily doing Python stuff on work time. Conversation on the bug is better.

There shouldn't be any need for the from err - chaining will happen automatically (that syntax is there for explicit non-trivial chaining). It's probably also a good idea to separate the attribute access from the invocation to ensure we wrap up the right exception:

try:
    size_index = size.__index__
except AttributeError:
    raise TypeError(f"{size!r} is not int-like enough")
else:
    size = size_index()   # now AttributeErrors from inside __index__ will be raised cleanly

res = bytearray()
while size < 0 or len(res) < size:
b = self.read(nreadahead())
Expand Down Expand Up @@ -868,6 +871,11 @@ def read(self, size=-1):
raise ValueError("read from closed file")
if size is None:
size = -1
else:
try:
size = size.__index__()
except AttributeError as err:
raise TypeError("an integer is required") from err
if size < 0:
size = len(self._buffer)
if len(self._buffer) <= self._pos:
Expand Down Expand Up @@ -905,7 +913,7 @@ def seek(self, pos, whence=0):
if self.closed:
raise ValueError("seek on closed file")
try:
pos.__index__
pos = pos.__index__()
except AttributeError as err:
raise TypeError("an integer is required") from err
if whence == 0:
Expand All @@ -932,7 +940,7 @@ def truncate(self, pos=None):
pos = self._pos
else:
try:
pos.__index__
pos = pos.__index__()
except AttributeError as err:
raise TypeError("an integer is required") from err
if pos < 0:
Expand Down Expand Up @@ -2357,11 +2365,12 @@ def read(self, size=None):
self._checkReadable()
if size is None:
size = -1
else:
try:
size = size.__index__()
except AttributeError as err:
raise TypeError("an integer is required") from err
decoder = self._decoder or self._get_decoder()
try:
size.__index__
except AttributeError as err:
raise TypeError("an integer is required") from err
if size < 0:
# Read everything.
result = (self._get_decoded_chars() +
Expand Down Expand Up @@ -2392,8 +2401,11 @@ def readline(self, size=None):
raise ValueError("read from closed file")
if size is None:
size = -1
elif not isinstance(size, int):
raise TypeError("size must be an integer")
else:
try:
size = size.__index__()
except AttributeError as err:
raise TypeError("an integer is required") from err

# Grab all the decoded text (we will rewind any extra bits later).
line = self._get_decoded_chars()
Expand Down
28 changes: 28 additions & 0 deletions Lib/test/test_memoryio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
import pickle
import sys

class IntLike():

@zooba zooba Aug 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No parens after class names

@orenmn orenmn Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad.
BTW, do you think this should be added to PEP8, or is it obvious?
(i grepped and found only 7 places in the codebase doing this.)

@zooba zooba Aug 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I found more than that, though they're all in tests. Maybe it's just my personal preference - don't worry about it if you don't want to

@orenmn orenmn Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not worried at all, and I would certainly follow your advice.
just wondered whether this should be added to the PEP..

and it seems that the docs think like you - https://docs.python.org/3.7/tutorial/classes.html#class-definition-syntax.

def __init__(self, num):
self._num = num
def __index__(self):
return self._num
__int__ = __index__

class MemorySeekTestMixin:

def testInit(self):
Expand Down Expand Up @@ -116,7 +123,10 @@ def test_truncate(self):
memio = self.ioclass(buf)

self.assertRaises(ValueError, memio.truncate, -1)
self.assertRaises(ValueError, memio.truncate, IntLike(-1))
memio.seek(6)
self.assertEqual(memio.truncate(IntLike(8)), 8)
self.assertEqual(memio.getvalue(), buf[:8])
self.assertEqual(memio.truncate(), 6)
self.assertEqual(memio.getvalue(), buf[:6])
self.assertEqual(memio.truncate(4), 4)
Expand All @@ -131,6 +141,7 @@ def test_truncate(self):
self.assertRaises(TypeError, memio.truncate, '0')
memio.close()
self.assertRaises(ValueError, memio.truncate, 0)
self.assertRaises(ValueError, memio.truncate, IntLike(0))

def test_init(self):
buf = self.buftype("1234567890")
Expand All @@ -154,12 +165,19 @@ def test_read(self):
self.assertEqual(memio.read(900), buf[5:])
self.assertEqual(memio.read(), self.EOF)
memio.seek(0)
self.assertEqual(memio.read(IntLike(0)), self.EOF)
self.assertEqual(memio.read(IntLike(1)), buf[:1])
self.assertEqual(memio.read(IntLike(4)), buf[1:5])
self.assertEqual(memio.read(IntLike(900)), buf[5:])
memio.seek(0)
self.assertEqual(memio.read(), buf)
self.assertEqual(memio.read(), self.EOF)
self.assertEqual(memio.tell(), 10)
memio.seek(0)
self.assertEqual(memio.read(-1), buf)
memio.seek(0)
self.assertEqual(memio.read(IntLike(-1)), buf)
memio.seek(0)
self.assertEqual(type(memio.read()), type(buf))
memio.seek(100)
self.assertEqual(type(memio.read()), type(buf))
Expand All @@ -169,6 +187,8 @@ def test_read(self):
memio.seek(len(buf) + 1)
self.assertEqual(memio.read(1), self.EOF)
memio.seek(len(buf) + 1)
self.assertEqual(memio.read(IntLike(1)), self.EOF)
memio.seek(len(buf) + 1)
self.assertEqual(memio.read(), self.EOF)
memio.close()
self.assertRaises(ValueError, memio.read)
Expand All @@ -178,6 +198,7 @@ def test_readline(self):
memio = self.ioclass(buf * 2)

self.assertEqual(memio.readline(0), self.EOF)
self.assertEqual(memio.readline(IntLike(0)), self.EOF)
self.assertEqual(memio.readline(), buf)
self.assertEqual(memio.readline(), buf)
self.assertEqual(memio.readline(), self.EOF)
Expand All @@ -186,9 +207,16 @@ def test_readline(self):
self.assertEqual(memio.readline(5), buf[5:10])
self.assertEqual(memio.readline(5), buf[10:15])
memio.seek(0)
self.assertEqual(memio.readline(IntLike(5)), buf[:5])
self.assertEqual(memio.readline(IntLike(5)), buf[5:10])
self.assertEqual(memio.readline(IntLike(5)), buf[10:15])
memio.seek(0)
self.assertEqual(memio.readline(-1), buf)
memio.seek(0)
self.assertEqual(memio.readline(IntLike(-1)), buf)
memio.seek(0)
self.assertEqual(memio.readline(0), self.EOF)
self.assertEqual(memio.readline(IntLike(0)), self.EOF)
# Issue #24989: Buffer overread
memio.seek(len(buf) * 2 + 1)
self.assertEqual(memio.readline(), self.EOF)
Expand Down
73 changes: 23 additions & 50 deletions Modules/_io/bytesio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ class _io.BytesIO "bytesio *" "&PyBytesIO_Type"
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=7f50ec034f5c0b26]*/

/*[python input]
class io_ssize_t_converter(CConverter):
type = 'Py_ssize_t'
converter = '_PyIO_ConvertSsize_t'
[python start generated code]*/
/*[python end generated code: output=da39a3ee5e6b4b0d input=d0a811d3cbfd1b33]*/

typedef struct {
PyObject_HEAD
PyObject *buf;
Expand Down Expand Up @@ -374,7 +381,7 @@ read_bytes(bytesio *self, Py_ssize_t size)

/*[clinic input]
_io.BytesIO.read
size as arg: object = None
size: io_ssize_t = -1
/

Read at most size bytes, returned as a bytes object.
Expand All @@ -384,28 +391,13 @@ Return an empty bytes object at EOF.
[clinic start generated code]*/

static PyObject *
_io_BytesIO_read_impl(bytesio *self, PyObject *arg)
/*[clinic end generated code: output=85dacb535c1e1781 input=cc7ba4a797bb1555]*/
_io_BytesIO_read_impl(bytesio *self, Py_ssize_t size)
/*[clinic end generated code: output=9cc025f21c75bdd2 input=c81ec53b8f2cc3cf]*/
{
Py_ssize_t size, n;
Py_ssize_t n;

CHECK_CLOSED(self);

if (PyLong_Check(arg)) {
size = PyLong_AsSsize_t(arg);
if (size == -1 && PyErr_Occurred())
return NULL;
}
else if (arg == Py_None) {
/* Read until EOF is reached, by default. */
size = -1;
}
else {
PyErr_Format(PyExc_TypeError, "integer argument expected, got '%s'",
Py_TYPE(arg)->tp_name);
return NULL;
}

/* adjust invalid sizes */
n = self->string_size - self->pos;
if (size < 0 || size > n) {
Expand All @@ -420,7 +412,7 @@ _io_BytesIO_read_impl(bytesio *self, PyObject *arg)

/*[clinic input]
_io.BytesIO.read1
size: object(c_default="Py_None") = -1
size: io_ssize_t = -1
/

Read at most size bytes, returned as a bytes object.
Expand All @@ -430,15 +422,15 @@ Return an empty bytes object at EOF.
[clinic start generated code]*/

static PyObject *
_io_BytesIO_read1_impl(bytesio *self, PyObject *size)
/*[clinic end generated code: output=a60d80c84c81a6b8 input=0951874bafee8e80]*/
_io_BytesIO_read1_impl(bytesio *self, Py_ssize_t size)
/*[clinic end generated code: output=d0f843285aa95f1c input=67cf18b142111664]*/
{
return _io_BytesIO_read_impl(self, size);
}

/*[clinic input]
_io.BytesIO.readline
size as arg: object = None
size: io_ssize_t = -1
/

Next line from the file, as a bytes object.
Expand All @@ -449,28 +441,13 @@ Return an empty bytes object at EOF.
[clinic start generated code]*/

static PyObject *
_io_BytesIO_readline_impl(bytesio *self, PyObject *arg)
/*[clinic end generated code: output=1c2115534a4f9276 input=ca31f06de6eab257]*/
_io_BytesIO_readline_impl(bytesio *self, Py_ssize_t size)
/*[clinic end generated code: output=4bff3c251df8ffcd input=7c95bd3f9e9d1646]*/
{
Py_ssize_t size, n;
Py_ssize_t n;

CHECK_CLOSED(self);

if (PyLong_Check(arg)) {
size = PyLong_AsSsize_t(arg);
if (size == -1 && PyErr_Occurred())
return NULL;
}
else if (arg == Py_None) {
/* No size limit, by default. */
size = -1;
}
else {
PyErr_Format(PyExc_TypeError, "integer argument expected, got '%s'",
Py_TYPE(arg)->tp_name);
return NULL;
}

n = scan_eol(self, size);

return read_bytes(self, n);
Expand Down Expand Up @@ -597,19 +574,15 @@ _io_BytesIO_truncate_impl(bytesio *self, PyObject *arg)
CHECK_CLOSED(self);
CHECK_EXPORTS(self);

if (PyLong_Check(arg)) {
size = PyLong_AsSsize_t(arg);
if (size == -1 && PyErr_Occurred())
return NULL;
}
else if (arg == Py_None) {
if (arg == Py_None) {
/* Truncate to current position if no argument is passed. */
size = self->pos;
}
else {
PyErr_Format(PyExc_TypeError, "integer argument expected, got '%s'",
Py_TYPE(arg)->tp_name);
return NULL;
size = PyNumber_AsSsize_t(arg, PyExc_OverflowError);
if (size == -1 && PyErr_Occurred()) {
return NULL;
}
}

if (size < 0) {
Expand Down
Loading