-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-29981: Update Index for set, dict, and generator 'comprehensions' #20272
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
Conversation
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.
Thanks for improving the documentation @DahlitzFlorian !
Co-authored-by: Rémi Lapeyre <[email protected]>
Thanks @remilapeyre for the review! I applied some of your suggestions. However, I would not change Use to Using as Use is the imperative form and highly preferred. See also: https://github.com/python/cpython/pull/995/files#r120774265 |
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 text reads well. I only noticed two minor grammar corrections. Thanks!
Thanks for the review @akuchling, I applied the requested changes. |
Thanks for accepting the PR @akuchling! Is there anything left to do or can it be merged? |
@DahlitzFlorian: on re-reviewing this, can you please move the 'Sets/Dicts can be created...' paragraphs in |
@akuchling changed it accordingly, thanks for the hint! |
Without looking at the resulting html, the additions look good to me. I think this should have a news entry such as: |
I think most documentation changes don't need a news entry, unless they're doing something large like adding a section or a whole new document. "Improved docs on topic X" doesn't tell you very much, and users don't need to modify code to accommodate doc changes. |
Thanks @DahlitzFlorian for the PR, and @akuchling for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
GH-22836 is a backport of this pull request to the 3.9 branch. |
… comprehensions'(pythonGH-20272) Co-authored-by: Rémi Lapeyre <[email protected]> (cherry picked from commit 2d55aa9) Co-authored-by: Florian Dahlitz <[email protected]>
GH-22837 is a backport of this pull request to the 3.8 branch. |
… comprehensions'(pythonGH-20272) Co-authored-by: Rémi Lapeyre <[email protected]> (cherry picked from commit 2d55aa9) Co-authored-by: Florian Dahlitz <[email protected]>
I have a lower threshhold -- is something added that users might want to read. But whoever does the work to push a PR over the finish line gets to decide. |
Thanks for the guidance @remilapeyre and @akuchling 🎉 |
… comprehensions'(GH-20272) Co-authored-by: Rémi Lapeyre <[email protected]> (cherry picked from commit 2d55aa9) Co-authored-by: Florian Dahlitz <[email protected]>
… comprehensions'(GH-20272) Co-authored-by: Rémi Lapeyre <[email protected]> (cherry picked from commit 2d55aa9) Co-authored-by: Florian Dahlitz <[email protected]>
… comprehensions'(pythonGH-20272) Co-authored-by: Rémi Lapeyre <[email protected]>
A similar PR (#995) was made back in 2017, but was closed due to inactivity. With this PR I propose nearly the same changes.
@terryjreedy mentioned that he does not like the "all or part of" phrase. As the already existing list comprehension's glossary entry has a similar phrasing, I suggest to keep it.
I build the changes locally and the index was updated accordingly.
https://bugs.python.org/issue29981