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
gh-82678: Fix parameter name for pipes #92016
base: main
Are you sure you want to change the base?
Conversation
fp = t.open(file, mode) | ||
where mode is 'r' to read the file, or 'w' to write it -- just like | ||
fp = t.open(file, rw) | ||
where rw is 'r' to read the file, or 'w' to write it -- just like |
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.
where rw is 'r' to read the file, or 'w' to write it -- just like | |
where 'rw' is 'r' to read the file, or 'w' to write it -- just like |
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.
Hmm, I wonder if making it a string could be misleading - without quotes, it's clear that it's a variable of some sort, while with, maybe it gets confused as a value
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.
A compromise could be:
where rw is 'r' to read the file, or 'w' to write it -- just like | |
where *rw* is 'r' to read the file, or 'w' to write it -- just like |
But the way @slateny has it currently is in keeping with the format used in the rest of the docstring: infile
and outfile
on line 46 are not surrounded by quotes or asterisks.
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.
Ah asterisks are a good idea, and good point on the consistency. I'm fine with both so I'll defer the choice here
@slateny The argument
|
Not sure what the conventions are when using type annotations - I just did the smallest change possible to fix the bug. I imagine that it's not noted directly under the method docstring because the implementation seems straightforward enough |
#82678
https://github.com/python/cpython/blob/main/Lib/pipes.py#L145
(Also a random
# '
here that could optionally be removed)