◐ Shell
clean mode source ↗

gh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic by erlend-aasland · Pull Request #104355 · python/cpython

…linic

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

arhadthedev

kumaraditya303

kumaraditya303

kumaraditya303

kumaraditya303

@erlend-aasland

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

AlexWaygood

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!

kumaraditya303

carljm added a commit to carljm/cpython that referenced this pull request

May 10, 2023

vstinner

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.


cls: defining_class
/
*args: object

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.

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

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

Choose a reason for hiding this comment

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

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