-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
bpo-46290: Fix parameter names in dataclasses docs #30450
Conversation
Misc/NEWS.d/next/Documentation/2022-01-07-10-23-56.bpo-46290.UKZFmh.rst
Outdated
Show resolved
Hide resolved
…KZFmh.rst Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@@ -406,10 +406,10 @@ Module contents | |||
def add_one(self): | |||
return self.x + 1 | |||
|
|||
.. function:: replace(instance, /, **changes) | |||
.. function:: replace(obj, /, **changes) |
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.
FTR the slash marks the arguments before it as positional-only, so the name chosen does not matter as it does not apply to code, it’s only for readers of the doc. (But if all other instances are changed, it makes sense to change this one too.)
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.
Yeah this is more for consistency than anything else.
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.
The first argument for the other functions, asdict()
, astuple()
, and isdataclass()
, is not positional-only. So changing the argument name is indeed a meaningful fix that should be made. In that context, indeed changing the name for replace()
for consistency makes sense.
Something the NEWS check doesn't know is that we don't add NEWS entries for documentation-only fixes. I'm adding the appropriate label to this PR and removing the NEWS entry. |
Thanks! I saw somewhere that minor typo changes generally don't need a NEWS entry but when the CI check failed I wasn't sure if I read outdated info or the CI check is wrong :) |
(cherry picked from commit ef5376e) Co-authored-by: Zsolt Dollenstein <zsol.zsol@gmail.com>
GH-30482 is a backport of this pull request to the 3.10 branch. |
(cherry picked from commit ef5376e) Co-authored-by: Zsolt Dollenstein <zsol.zsol@gmail.com>
GH-30483 is a backport of this pull request to the 3.9 branch. |
Thanks for the PR, @zsol! |
Fix documentation typos in parameter names for dataclasses API.
These are important to avoid confusion when passing in arguments as keywords, like in Instagram/LibCST#585 (comment)
https://bugs.python.org/issue46290