GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-90533: Implement BytesIO.peek() by marcelm · Pull Request #30808 · python/cpython

Conversation

@marcelm

@marcelm marcelm commented Jan 22, 2022 •
edited by AlexWaygood
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Closes gh-90533

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mhils and dunielpls reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Jan 22, 2022
@the-knights-who-say-ni

This comment was marked as outdated.

Sign in to view
@bedevere-bot bedevere-bot added the awaiting review label Jan 22, 2022
@the-knights-who-say-ni the-knights-who-say-ni added CLA signed and removed CLA not signed labels Feb 1, 2022
@marcelm
marcelm force-pushed the fix-issue-46375 branch from 3111318 to 675718d Compare April 11, 2022 20:06
@marcelm marcelm changed the title bpo-46375: Implement BytesIO.peek() gh-90533: Implement BytesIO.peek() Apr 11, 2022
@marcelm

This comment was marked as outdated.

Sign in to view
@AlexWaygood AlexWaygood added the topic-IO label Apr 14, 2022
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@marcelm
marcelm force-pushed the fix-issue-46375 branch from 675718d to d4e849f Compare August 5, 2022 22:44
@bedevere-bot

bedevere-bot commented Aug 5, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marcelm
marcelm force-pushed the fix-issue-46375 branch from ef5eb69 to b82fa85 Compare August 5, 2022 23:28
@marcelm
marcelm force-pushed the fix-issue-46375 branch 2 times, most recently from 6998be1 to be39ff2 Compare November 6, 2022 15:11
@marcelm

marcelm commented Nov 9, 2022

Copy link
Copy Markdown
Contributor Author

@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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood AlexWaygood added the type-feature A feature request or enhancement label Nov 9, 2022
@AlexWaygood

AlexWaygood commented Nov 9, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
AlexWaygood self-requested a review November 9, 2022 13:37
@marcelm

marcelm commented Nov 9, 2022

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303
kumaraditya303 requested a review from benjaminp November 27, 2022 07:18
@AlexWaygood

AlexWaygood commented Nov 28, 2022

Copy link
Copy Markdown
Member

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
AlexWaygood removed their request for review November 28, 2022 12:26
@marcelm

marcelm commented Nov 28, 2022

Copy link
Copy Markdown
Contributor Author

Thank you! I appreciate that you took the time.
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@awalgarg

awalgarg commented Jul 8, 2023

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marcelm

marcelm commented Jul 8, 2023

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@awalgarg

awalgarg commented Jul 8, 2023

Copy link
Copy Markdown

@marcelm That was quick, thank you so much!
marcelm reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marcelm
marcelm force-pushed the fix-issue-46375 branch from 5cb3143 to 79d5032 Compare September 19, 2023 08:04
@AA-Turner

AA-Turner commented Sep 21, 2023

Copy link
Copy Markdown
Member

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

A
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Sep 21, 2023
View reviewed changes
Comment thread Doc/library/io.rst Outdated
.. versionadded:: 3.13

Return bytes from the current position onwards but without advancing the
position. The number of bytes returned may be less or more than requested.

@vstinner vstinner Sep 21, 2023

Copy link
Copy Markdown
Member

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.

Wait. Why can it more than requested?

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@marcelm marcelm Sep 22, 2023

Copy link
Copy Markdown
Contributor Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Sep 28, 2023

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@marcelm marcelm Sep 28, 2023

Copy link
Copy Markdown
Contributor Author

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.

Absolutely, it’s already changed.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/_pyio.py Outdated
pos = self.tell()
if size == 0:
size = -1
b = self.read(size)

@vstinner vstinner Sep 21, 2023

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@marcelm marcelm Sep 22, 2023

Copy link
Copy Markdown
Contributor Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_memoryio.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@marcelm
marcelm requested review from a team, 1st1, DinoV, Fidget-Spinner, StanFromIreland, Yhg1s, ZeroIntensity, asvetlov, brandtbucher, brettcannon, corona10, diegorusso, ericsnowcurrently, ericvsmith, hugovk, markshannon, pfmoore, pganssle, picnixz, pradyunsg, savannahostrowski and willingc as code owners June 4, 2026 12:50
@python-cla-bot

python-cla-bot Bot commented Jun 4, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Jun 4, 2026

Copy link
Copy Markdown
Member

Commits 533
Files changed 1299

Oh oh. Something went wrong with this PR :-( I suggest to abandon this PR and create a new one.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marcelm

marcelm commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, I merged main manually; it seems this triggered a lot of stuff I did not intend to.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marcelm
Do not cap at DEFAULT_BUFFER_SIZE
32fd791
@marcelm

marcelm commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

I will open a new PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@marcelm marcelm mentioned this pull request Jun 4, 2026
gh-90533: Implement BytesIO.peek() #150917
Merged
@vstinner

vstinner commented Jun 4, 2026

Copy link
Copy Markdown
Member

The new PR: #150917.
marcelm reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Jun 4, 2026

Copy link
Copy Markdown
Member

@marcelm, probably you forgot to pull changes from upstream to your main branch or to your feature branch.
marcelm reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
@cmaloney cmaloney cmaloney approved these changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@erlend-aasland erlend-aasland erlend-aasland left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting merge topic-IO type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

io.BytesIO does not have peek()

Uh oh!

There was an error while loading. Please reload this page.

14 participants

@marcelm @the-knights-who-say-ni @bedevere-bot @AlexWaygood @awalgarg @AA-Turner @vstinner @erlend-aasland @pitrou @cmaloney @emmatyping @hugovk @serhiy-storchaka @ezio-melotti
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.