gh-90533: Implement BytesIO.peek()

[marcelm]

Closes gh-90533

[bedevere-bot]

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

[marcelm]

@AlexWaygood You’ve been the only human to interact with this PR so far, do you possibly have any advice on how to move this forward?

[AlexWaygood]

Hi @marcelm — sorry for the delay in anybody looking at this. I haven't studied your PR in detail (or thought about whether the proposal is a good idea), but it looks well put together at first glance.

I'll try to take a look soon. I won't be able to review the C code, but I can comment on whether the proposal seems like a good idea, and I can review the Python implementation and the tests.

Note that if this proposal is accepted, it will also need:

• Updates to the docs for BytesIO (Doc/library/io.rst)
• An entry in "What's new in Python 3.12" (Doc/whatsnew/3.12.rst)

You can also add yourself to Misc/ACKS as part of this PR, and give yourself credit in the NEWS entry, if you like. (Neither is compulsory.)

[marcelm]

Thank you! I pushed a documentation update and will add an entry to the What’s new document in case the PR is reviewed favorably.

[AlexWaygood]

I'll defer to @benjaminp's judgement on this one -- I'm really not the right reviewer for this, unfortunately :(

Please ping me again if you still haven't had a review in a few weeks.

[marcelm]

Thank you! I appreciate that you took the time.

[awalgarg]

Please ping me again if you still haven't had a review in a few weeks.

@AlexWaygood Hi! Been 8 months, no review so far :) How can we take this forward? I'm not the original author of the PR but I also have a use-case for this API and would like to see this change land, happy to help progress this.

[marcelm]

Thanks for your interest! I have updated the PR to fix the merge conflict and to reflect that it now needs to target 3.13.

[awalgarg]

@marcelm That was quick, thank you so much!

[AA-Turner]

cc: @erlend-aasland @vstinner @benjaminp for the io C code.

A

[vstinner]

Wait. Why can it more than requested?

Apparently size=0 means "read all". Please document it.

[marcelm]

This was just copied from the documentation for BufferedReader.peek(), but I agree that it could be written better. Changed now.

Wait. Why can it more than requested?

BufferedReader.peek() ignores the size argument and just returns whatever it has in its internal buffer.

[vstinner]

I would prefer that BytesIO.peek() documents its behavior, rather than BufferedReader.peek() behavior.

[marcelm]

Absolutely, it’s already changed.

(BTW, is there a policy on who clicks the "Resolve conversation" button?)

[vstinner]

Would it be possible to implement it without touching the position? This code is not thread safe. I don't know if it's supposed to be thread-safe. Maybe add a private read method which has an argument to decide to move the position or not.

Same remark for C code.

[marcelm]

Good point; I implemented this now so that the position is not changed. I factored out a peek_bytes function in the C version that does not advance the position.

It seems though that neither the C nor the Python version of BytesIO are supposed to be thread safe. (They don’t use locks as e.g. BufferedReader does.) So I would suggest making them so would be a task for a different PR.

[vstinner]

I asked BytesIO.peek(0) to return an empty string, but I didn't notice that BufferedReader.peek(0) returns non-empty string. Now I'm unhappy that the two classes have an inconsistent behavior on peek(0), sorry.

>>> f=open("/etc/issue", "rb")
>>> f.peek(0)
b'\\S\nKernel \\r on \\m (\\l)\n\n'

On a short file, BufferedReader.peek(0) even returned the whole file contents!

Since BufferedReader.peek(0) behavior already exists, I would suggest to use the same behavior on BytesIO.peek(): return non-empty string.

cc @cmaloney

[marcelm]

Since BufferedReader.peek(0) behavior already exists, I would suggest to use the same behavior on BytesIO.peek(): return non-empty string.

Sure! I have changed it so that the rest of the buffer is returned for size=0.

The signature is still such that the default is size=1. Maybe that is actually a good compromise?

• If desired, an explicit peek(0) returns a full (possibly expensive) copy - but the default is cheap.
• peek() without arguments returns a single byte, which is the minimum necessary for compatibility with BufferedReader.peek(). Note that the latter also can return a single byte when that is how much is left in the buffer:

$ python3 -c 'f=open("README.rst", "rb");f.read(8191);print(f.peek())'
b'r'

• If more than a single byte but not the full buffer is needed, an explicit size can be set.

Alternatively, the default could become size=0, but then the question is how huch of the buffer to return (which is where this PR got stalled last time).