-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-90533: Implement BytesIO.peek() #30808
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.
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
base: main
Are you sure you want to change the base?
Changes from 30 commits
b833b83
50a2cfb
eaa7672
00457ae
882579d
c1eed72
afc200c
79ab9a4
b493914
2a1c85c
d398717
26d1e81
9a19ff9
9300ade
d214089
d6691b8
3e51adb
3661b65
cd40d77
04372bd
6b9ae8c
f7406f6
d9528e2
bc8134b
b6ffca8
5fe5645
4126a64
1ea40c2
77e04d6
4d2f2dd
c16bebf
08bd7da
6174fca
b8b8cf4
7ac914e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -566,6 +566,48 @@ def test_issue141311(self): | |
| buf = bytearray(2) | ||
| self.assertEqual(0, memio.readinto(buf)) | ||
|
|
||
| def test_peek(self): | ||
| buf = self.buftype("1234567890") | ||
| with self.ioclass(buf) as memio: | ||
|
marcelm marked this conversation as resolved.
Outdated
|
||
| self.assertEqual(memio.tell(), 0) | ||
| self.assertEqual(memio.peek(1), buf[:1]) | ||
| self.assertEqual(memio.peek(1), buf[:1]) | ||
|
vstinner marked this conversation as resolved.
|
||
| self.assertEqual(memio.peek(), buf[:1]) | ||
| self.assertEqual(memio.peek(3), buf[:3]) | ||
| self.assertEqual(memio.peek(5), buf[:5]) | ||
| self.assertEqual(memio.peek(0), b"") | ||
| self.assertEqual(memio.peek(len(buf) + 100), buf) | ||
| self.assertEqual(memio.peek(-1), buf) | ||
| self.assertEqual(memio.tell(), 0) | ||
| memio.read(1) | ||
| self.assertEqual(memio.tell(), 1) | ||
| self.assertEqual(memio.peek(1), buf[1:2]) | ||
| self.assertEqual(memio.peek(), buf[1:2]) | ||
| self.assertEqual(memio.peek(3), buf[1:4]) | ||
| self.assertEqual(memio.peek(5), buf[1:6]) | ||
| self.assertEqual(memio.peek(0), b"") | ||
| self.assertEqual(memio.peek(len(buf) + 100), buf[1:]) | ||
| self.assertEqual(memio.peek(-1), buf[1:]) | ||
| self.assertEqual(memio.tell(), 1) | ||
| memio.read() | ||
| self.assertEqual(memio.tell(), len(buf)) | ||
| self.assertEqual(memio.peek(1), self.EOF) | ||
| self.assertEqual(memio.peek(3), self.EOF) | ||
| self.assertEqual(memio.peek(5), self.EOF) | ||
| self.assertEqual(memio.peek(0), b"") | ||
| self.assertEqual(memio.tell(), len(buf)) | ||
| # Peeking works after writing | ||
| abc = self.buftype("abc") | ||
| memio.write(abc) | ||
| self.assertEqual(memio.peek(), self.EOF) | ||
| memio.seek(len(buf)) | ||
| self.assertEqual(memio.peek(), abc[:1]) | ||
| self.assertEqual(memio.peek(-1), abc) | ||
| self.assertEqual(memio.peek(len(abc) + 100), abc) | ||
| self.assertEqual(memio.tell(), len(buf)) | ||
|
marcelm marked this conversation as resolved.
|
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you test that seek to end of file then peek() returns (Think it's right in implementation, just "no data was read because whole buffer skipped" can catch different cases from "no data was read but buffer has been touched")
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added within a new |
||
| self.assertRaises(ValueError, memio.peek) | ||
|
|
||
| def test_unicode(self): | ||
| memio = self.ioclass() | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Add :meth:`io.BytesIO.peek`. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not default to
size=0likeBufferedReader?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read through the comments. In general anything returning a
bytesrather than abytes-likememoryview is going to require allocating and copying potentially many bytes... If the copy is really concerning I'd lean returning a memoryview rather than abyteswhich is a mandatory copy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memoryviewcame up in this comment and the three following it: #30808 (comment)I don’t know what the conclusion is given your comment. Should a memoryview be returned instead? Most important to me is compatibility with what
BufferedReader.peek()returns.I am not too concerned about the extra memory for a
bytesobject as long as the default issize=1.