FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-124478: Cleanup argparse documentation by savannahostrowski · Pull Request #124877 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

GH-124478: Cleanup argparse documentation - #124877

Merged
JelleZijlstra merged 24 commits into
python:mainfrom
savannahostrowski:gh-124478
Oct 8, 2024
ConversationCommits (24)ChecksFiles changed
Merged

GH-124478: Cleanup argparse documentation#124877
JelleZijlstra merged 24 commits into
python:mainfrom
savannahostrowski:gh-124478

Conversation

savannahostrowski commented Oct 2, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

This PR:

  1. Removes duplicative examples that are fairly basic and sufficiently covered in the tutorial
  2. Removes the quick reference tables, as removing duplicative examples moves the reference docs for ArgumentParser and add_argument much higher up the page. I don't think this is terribly necessary anymore.
  3. Move the upgrading to optparse guide to a separate page. Since optparse has been deprecated since 3.2, this is probably a minimally viewed part of the main argparse docs, so I think it's fair to have it on a separate page.
  4. Consolidates some examples
  5. Updates terminology (e.g., sub-command -> subcommand) for consistency.

Please let me know if there are other best practices we consistently use in other documentation areas but lack here. I'm happy to incorporate them here so we can get these documents feeling fresh!

(As a note, I tried to be somewhat conservative here as I do think many of the examples are 1) valuable for new users and 2) too advanced to add to the tutorial in its current form)

bedevere-app Bot mentioned this pull request Oct 2, 2024
Closed
bedevere-app Bot added docs Documentation in the Doc dir skip news awaiting review labels Oct 2, 2024
JelleZijlstra reviewed Oct 2, 2024
View reviewed changes
Comment thread Doc/library/argparse-optparse.rst
Comment thread Doc/library/argparse-optparse.rst Outdated
Comment thread Doc/library/argparse-optparse.rst Outdated
savannahostrowski and others added 3 commits October 1, 2024 20:26
savannahostrowski commented Oct 2, 2024
View reviewed changes
Comment thread Doc/library/argparse.rst Outdated
JelleZijlstra reviewed Oct 2, 2024
View reviewed changes
Comment thread Doc/library/argparse.rst
Comment thread Doc/library/argparse.rst
Comment thread Doc/library/argparse.rst Outdated

JelleZijlstra commented Oct 2, 2024

Copy link
Copy Markdown
Member

I looked at the preview https://cpython-previews--124877.org.readthedocs.build/en/124877/library/argparse.html#module-argparse and skimmed the entire doc. Below are some things I noticed. None are new to this PR, so feel free to ignore them and I can file a separate PR with those cleanups.

  • It's distracting that the first few paragraphs link :mod:`argparse` 4 times, and it just links back to the top of the page. Would recommend suppressing those links with :mod:`!argparse`.
  • RawTextHelpFormatter description says "new lines"; I think it's normally spelled "newlines".
  • "ArgumentParser uses filesystem encoding and error handler to read the file containing arguments." this sentence reads oddly, maybe needs a "the"
  • "the action default will not over write it" -> I think "overwrite" is one word
  • I might remove the parser.add_argument('source_file', type=open) example. It doesn't make it easy to properly close the file, and as the text says later, FileNotFoundError would not be handled nicely.
  • FileType is formatted inconsistently in the paragraph that starts Even [FileType](https://cpython-previews--124877.org.readthedocs.build/en/124877/library/argparse.html#argparse.FileType) has its limitations for use with the type keyword.
  • The text after "class argparse.Action" should be indented by another level.
  • The docs for parse_known_intermixed_args are not properly indented.

savannahostrowski commented Oct 2, 2024

Copy link
Copy Markdown
Member Author

Thanks so much for such a thorough review, @JelleZijlstra. I think I can incorporate your feedback in this PR. I'll revisit this tomorrow evening.

tomasr8 reviewed Oct 2, 2024
View reviewed changes
Comment thread Doc/library/argparse.rst Outdated
Comment thread Doc/howto/argparse-optparse.rst Outdated
savannahostrowski and others added 2 commits October 2, 2024 08:30
Update argparse-optparse.rst
300ed64 
Co-authored-by: Tomas R <tomas.roun8@gmail.com>
Update argparse.rst
3273f03 
Co-authored-by: Tomas R <tomas.roun8@gmail.com>
willingc approved these changes Oct 2, 2024
View reviewed changes

willingc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Lovely work @savannahostrowski. Thank you!

savannahostrowski commented Oct 3, 2024

Copy link
Copy Markdown
Member Author

@JelleZijlstra, I applied all of your suggestions. I also found formatting inconsistencies in taking another pass related to indentation and style.

JelleZijlstra reviewed Oct 3, 2024
View reviewed changes
Comment thread Doc/library/argparse.rst Outdated
JelleZijlstra added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Oct 8, 2024
JelleZijlstra merged commit 37228bd into python:main Oct 8, 2024

miss-islington-app Bot commented Oct 8, 2024

Copy link
Copy Markdown

Thanks @savannahostrowski for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington-app Bot commented Oct 8, 2024

Copy link
Copy Markdown

Sorry, @savannahostrowski and @JelleZijlstra, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 37228bd16e3ef97d32da08848552f7ef016d68ab 3.13

miss-islington-app Bot commented Oct 8, 2024

Copy link
Copy Markdown

Sorry, @savannahostrowski and @JelleZijlstra, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 37228bd16e3ef97d32da08848552f7ef016d68ab 3.12

bedevere-app Bot commented Oct 8, 2024

Copy link
Copy Markdown

GH-125162 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label Oct 8, 2024
savannahostrowski added a commit to savannahostrowski/cpython that referenced this pull request Oct 8, 2024
[3.13] pythonGH-124478: Cleanup argparse documentation (pythonGH-124877)
6ba27ae 
(cherry picked from commit 37228bd)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Tomas R <tomas.roun8@gmail.com>

bedevere-app Bot commented Oct 8, 2024

Copy link
Copy Markdown

GH-125164 is a backport of this pull request to the 3.12 branch.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 8, 2024
JelleZijlstra added a commit that referenced this pull request Oct 8, 2024
[3.13] GH-124478: Cleanup argparse documentation (GH-124877) (#125162)
5f2a5ac 
(cherry picked from commit 37228bd)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Tomas R <tomas.roun8@gmail.com>
JelleZijlstra added a commit that referenced this pull request Oct 8, 2024
[3.12] GH-124478: Cleanup argparse documentation (GH-124877) (#125164)
0dbad1d 
(cherry picked from commit 37228bd)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Tomas R <tomas.roun8@gmail.com>
efimov-mikhail pushed a commit to efimov-mikhail/cpython that referenced this pull request Oct 9, 2024
pythonGH-124478: Cleanup argparse documentation (python#124877)
0dcbd9b 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Tomas R <tomas.roun8@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tomasr8 tomasr8 left review comments
JelleZijlstra JelleZijlstra approved these changes
willingc willingc approved these changes

Assignees

JelleZijlstra

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL