Skip to content
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

Jump to bottom

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

Merged
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 4 commits
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
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
Copy link
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.

Copy link
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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Steve?

Copy link
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():
Copy link
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

Copy link
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.)

Copy link
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

Copy link
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