Skip to content
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-87358: Add clarification about nargs and default argparse behaviour #124094

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Sep 14, 2024

After reading through the issue's comments, it seems like all that's needed here is additional clarification in the argparse docs about how nargs and default interact.


📚 Documentation preview 📚: https://cpython-previews--124094.org.readthedocs.build/

@savannahostrowski savannahostrowski added docs Documentation in the Doc dir stdlib Python modules in the Lib dir 3.14 new features, bugs and security fixes labels Sep 14, 2024
Comment on lines 1125 to 1126
For other nargs_ values, the argument is treated as a required argument and
the default_ value is ignored.
Copy link
Member

Choose a reason for hiding this comment

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

This looks misleading, because it is not clear that this only relates to positional arguments.

On other hand, default is also has no effect for required optional arguments.

We could say that the default value is ignored for required arguments (there may be a reference to the required attribute).

@gpshead
Copy link
Member

gpshead commented Sep 23, 2024

For future reference: Avoid rebase and force pushing as it causes codeowners assignment for all of the other changes rebased in. Just merge main in instead.

We all do this from time to time. (Even at work!) It's a GitHub git internals UX flaw. 😅

@savannahostrowski
Copy link
Member Author

Yeah, oof, that's my bad! Apologies for the noise, folks 😅.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes and removed 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir labels Sep 24, 2024
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

I am not sure about wording. LGTM, but it would be better if some of native speakers check it.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Wording is good, thanks!

@@ -1122,6 +1122,9 @@ is used when no command-line argument was present::
>>> parser.parse_args([])
Namespace(foo=42)

For required_ arguments, the ``default`` value is ignored. For example, this
Copy link
Member

Choose a reason for hiding this comment

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

Aside: we have a (somewhat unusual) convention of using italics for arguments instead of literal code formatting:

https://devguide.python.org/documentation/markup/

But I see we're using literals elsewhere in this file so it's best to be consistent.

@hugovk hugovk merged commit 20ccda0 into python:main Sep 24, 2024
23 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 24, 2024
…haviour (pythonGH-124094)

(cherry picked from commit 20ccda0)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 24, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 24, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 24, 2024
…haviour (pythonGH-124094)

(cherry picked from commit 20ccda0)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 24, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Sep 24, 2024
hugovk pushed a commit that referenced this pull request Sep 24, 2024
…ehaviour (GH-124094) (#124441)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
Yhg1s pushed a commit that referenced this pull request Sep 24, 2024
…ehaviour (GH-124094) (#124440)

GH-87358: Add clarification about nargs and default argparse behaviour (GH-124094)
(cherry picked from commit 20ccda0)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
@savannahostrowski savannahostrowski deleted the gh-87358 branch September 27, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants