Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-110383: Update dict get, fromkeys, and setdefault signatures in docs#119330
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-110383: Update dict get, fromkeys, and setdefault signatures in docs #119330
Uh oh!
There was an error while loading. Please reload this page.
Conversation
johnlandonwood commented May 21, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
ghost commented May 21, 2024 • edited by ghost
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by ghost
Uh oh!
There was an error while loading. Please reload this page.
encukou commented May 21, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Thank you! Please also remove the square brackets: those are another way of saying that the argument is optional, they aren't needed if the default value is given. This change makes the |
erlend-aasland commented May 21, 2024
Thanks for the PR! Changes like this are ... controversial. There's a group of core devs who prefer the Moreover, this PR introduces an inconsistency when we compare So, sorry, I think we have to reject this particular change for now. IMO, it should not be in the TODO list in the issue; changes like this are controversial, and thus no good match as a beginner PR/issue :( |
johnlandonwood commented May 21, 2024
@encukou and @erlend-aasland - thank you for the feedback! No worries, I wasn't aware of the controversy around those notations. I just saw it in that issue and it looked like a layup, but things are often more complex than they seem. I appreciate the context :) |
erlend-aasland commented May 21, 2024
I just had a talk with Petr, and I see now that this particular change is not in the controversial camp :) It does make sense to align the docs with the docstring; so let's land this! |
Uh oh!
There was an error while loading. Please reload this page.
…landonwood/cpython into pythongh-110383-update-dict-get-doc
johnlandonwood commented May 21, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Great to hear! I just updated |
johnlandonwood commented May 21, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Wait, one point of confusion - should the dosctring for |
get, pop, fromkeys, and setdefault signatures in docserlend-aasland commented May 21, 2024
Well, it is not that easy; you'd have to change the Argument Clinic input and regenerate the parsing code: Lines 4377 to 4395 in b7f45a9
So, that change would look like this1: diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 985a326a176..c976a0c868f 100644 --- a/Objects/dictobject.c+++ b/Objects/dictobject.c@@ -4378,7 +4378,7 @@ dict_clear_impl(PyDictObject *self) dict.pop key: object - default: object = NULL+ default: object = None / D.pop(k[,d]) -> v, remove specified key and return the corresponding value.Footnotes
|
erlend-aasland commented May 21, 2024
I think we should consider leaving the |
rhettinger commented May 21, 2024
Except for |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
get, pop, fromkeys, and setdefault signatures in docsget, fromkeys, and setdefault signatures in docserlend-aasland commented May 21, 2024
Thank you, @johnlandonwood, and congrats on landing your first PR :) |
johnlandonwood commented May 21, 2024
No, thanks to you and the others for what you do! Feels great! |
rhettinger commented May 21, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Hmm, I think I remember why these got skipped in all of the past sweeps. The issue was that the bracketless form implied that keyword arguments could be used. Also, there was a decision to avoid use Also, it looks like the actual help signatures sometimes look terrible and we don't want that to spill to the main docs: Go ahead and do want you want. I don't have an opinion. I just know that these were intentionally not changed during past sweeps. |
This comment has been minimized.
This comment has been minimized.
…s with docstrings (pythonGH-119330) (cherry picked from commit 0e3c8cd) Co-authored-by: Landon Wood <landon@elkrange.com>
…s with docstrings (pythonGH-119330) (cherry picked from commit 0e3c8cd) Co-authored-by: Landon Wood <landon@elkrange.com>
GH-119370 is a backport of this pull request to the 3.13 branch. |
GH-119371 is a backport of this pull request to the 3.12 branch. |
Small update to address "list of errata: 2" in #110383, by updating the signature of dict methods
get,pop,fromkeys, andsetdefaultin the docs.📚 Documentation preview 📚: https://cpython-previews--119330.org.readthedocs.build/