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

gh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic #104355

Merged
merged 6 commits into from
May 10, 2023
Merged

gh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic #104355

merged 6 commits into from
May 10, 2023

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented May 10, 2023 •
edited by bedevere-bot
Loading

Make sure the defining class is passed to all methods,
so we can easily fetch module state from them in the future.

@erlend-aasland
pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument C…
98dd91b 
…linic

Make sure the defining class is passed to all methods,
so we can easily fetch module state from them in the future.
@bedevere-bot bedevere-bot mentioned this pull request May 10, 2023
Closed
kumaraditya303
kumaraditya303 previously approved these changes May 10, 2023
View reviewed changes
@kumaraditya303 kumaraditya303 dismissed their stale review May 10, 2023 11:36

pressed wrong button

@erlend-aasland
Copy link
Contributor Author

@AlexWaygood, would you mind taking a look at the docstrings. I exploited this possibility to tweak them a little bit (consistent mood, etc.)

AlexWaygood
AlexWaygood reviewed May 10, 2023
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Docstrings look good -- two small points!

Modules/_io/bufferedio.c Outdated Show resolved Hide resolved
Modules/_io/bufferedio.c Outdated Show resolved Hide resolved
@erlend-aasland erlend-aasland merged commit ce8d3db into python:main May 10, 2023
@erlend-aasland erlend-aasland deleted the isolate-io/clinic branch May 10, 2023 14:23
carljm added a commit to carljm/cpython that referenced this pull request May 10, 2023
@carljm
Merge branch 'main' into fixloadsuperspec
5976df9 
* main:
  pythonGH-102181: Improve specialization stats for SEND (pythonGH-102182)
  pythongh-103000: Optimise `dataclasses.asdict` for the common case (python#104364)
  pythongh-103538: Remove unused TK_AQUA code (pythonGH-103539)
  pythonGH-87695: Fix OSError from `pathlib.Path.glob()` (pythonGH-104292)
  pythongh-104263: Rely on Py_NAN and introduce Py_INFINITY (pythonGH-104202)
  pythongh-104010: Separate and improve docs for `typing.get_origin` and `typing.get_args` (python#104013)
  pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic (python#104355)
  pythongh-103960: Dark mode: invert image brightness (python#103983)
  pythongh-104252: Immortalize Py_EMPTY_KEYS (pythongh-104253)
  pythongh-101819: Clean up _io windows console io after pythongh-104197 (python#104354)
  pythongh-101819: Harden _io init (python#104352)
  pythongh-103247: clear the module cache in a test in test_importlib/extensions/test_loader.py (pythonGH-104226)
  pythongh-103848: Adds checks to ensure that bracketed hosts found by urlsplit are of IPv6 or IPvFuture format (python#103849)
  pythongh-74895: adjust tests to work on Solaris (python#104326)
  pythongh-101819: Refactor _io in preparation for module isolation (python#104334)
  pythongh-90953: Don't use deprecated AST nodes in clinic.py (python#104322)
  pythongh-102327: Extend docs for "url" and "headers" parameters to HTTPConnection.request()
  pythongh-104328: Fix typo in ``typing.Generic`` multiple inheritance error message (python#104335)
vstinner
vstinner reviewed May 11, 2023
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Review after merge: LGTM.

The doc and docstring can be improved, but it's unrelated to this PR.

Modules/_io/bufferedio.c

cls: defining_class
/
*args: object
Copy link
Member

Choose a reason for hiding this comment

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

IMO the signature is wrong and should be fixed. It should have 1 optional positional argument, maybe called size. Look at other read() functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I did think about that. I also think AC should be able to attach Py_UNUSED to unused arguments (I'm working on that.) I'll create a follow-up PR for fixing the clinic input. (This is not the only problematic usage of *args in _io.)

Modules/_io/bufferedio.c
return all data until EOF.

If the argument is positive, and the underlying raw stream is
not 'interactive', multiple raw reads may be issued to satisfy
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to clarify what "interactive" means. I suppose that it's related to "is it a TTY?" test.

The doc uses the same phrasing: https://docs.python.org/dev/library/io.html#io.BufferedIOBase.read

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Let's improve that in a separate issue/PR. Good observation, though!

carljm added a commit to carljm/cpython that referenced this pull request May 11, 2023
@carljm
Merge branch 'main' into supermon
19b3d9e 
* main: (27 commits)
  pythongh-87849: fix SEND specialization family definition (pythonGH-104268)
  pythongh-101819: Adapt _io.IOBase.seek and _io.IOBase.truncate to Argument Clinic (python#104384)
  pythongh-101819: Adapt _io._Buffered* methods to Argument Clinic (python#104367)
  pythongh-101819: Refactor `_io` futher in preparation for module isolation (python#104369)
  pythongh-101819: Adapt _io.TextIOBase methods to Argument Clinic (python#104383)
  pythongh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs (python#104287)
  pythonGH-92184: Convert os.altsep to '/' in filenames when creating ZipInfo objects (python#92185)
  pythongh-104357: fix inlined comprehensions that close over iteration var (python#104368)
  pythonGH-90208: Suppress OSError exceptions from `pathlib.Path.glob()` (pythonGH-104141)
  pythonGH-102181: Improve specialization stats for SEND (pythonGH-102182)
  pythongh-103000: Optimise `dataclasses.asdict` for the common case (python#104364)
  pythongh-103538: Remove unused TK_AQUA code (pythonGH-103539)
  pythonGH-87695: Fix OSError from `pathlib.Path.glob()` (pythonGH-104292)
  pythongh-104263: Rely on Py_NAN and introduce Py_INFINITY (pythonGH-104202)
  pythongh-104010: Separate and improve docs for `typing.get_origin` and `typing.get_args` (python#104013)
  pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic (python#104355)
  pythongh-103960: Dark mode: invert image brightness (python#103983)
  pythongh-104252: Immortalize Py_EMPTY_KEYS (pythongh-104253)
  pythongh-101819: Clean up _io windows console io after pythongh-104197 (python#104354)
  pythongh-101819: Harden _io init (python#104352)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

@arhadthedev arhadthedev arhadthedev left review comments

@vstinner vstinner vstinner left review comments

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@erlend-aasland @vstinner @arhadthedev @kumaraditya303 @AlexWaygood @bedevere-bot