Skip to content

GH-122679: Add register() to argparse docs #126939

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Nov 22, 2024
Merged

GH-122679: Add register() to argparse docs #126939

merged 16 commits into from
Nov 22, 2024

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Nov 17, 2024 •
edited by github-actions bot
Loading

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting core review labels Nov 17, 2024
@bedevere-app bedevere-app bot mentioned this pull request Nov 17, 2024
Closed
@savannahostrowski
Copy link
Member Author

@serhiy-storchaka If you have bandwidth, I'd love it if you could look at this one since you weighed in on the original issue. I debated expanding the section about custom type converters in the Tutorial instead, but it felt more correct to properly document register(). We could omit the example here, but I feel like it provides some clarity on how to use the method.

@savannahostrowski savannahostrowski linked an issue Nov 17, 2024 that may be closed by this pull request
argparse: allow to override the type name in 'invalid %(type)s value: %(value)r' error messages #122679
Closed
danielhollas
danielhollas reviewed Nov 18, 2024
View reviewed changes
Copy link
Contributor

@danielhollas danielhollas left a comment

Choose a reason for hiding this comment

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

Couple quick comments from somebody who doesn't have a deep knowledge of argparse internals (so feel free to ignore:-))

serhiy-storchaka
serhiy-storchaka reviewed Nov 18, 2024
View reviewed changes
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.

The argparse documentation is larger and may even look bloated. Please try to use more terse style.

Please add references to registry() in the parts that describe custom type and action. Currently it it not even documented that the type argument may be a name, not callable.

serhiy-storchaka
serhiy-storchaka reviewed Nov 19, 2024
View reviewed changes
serhiy-storchaka
serhiy-storchaka approved these changes Nov 21, 2024
View reviewed changes
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.

LGTM. 👍

@savannahostrowski
Copy link
Member Author

@brandtbucher as FYI :)

@savannahostrowski savannahostrowski merged commit fcfdb55 into python:main Nov 22, 2024
29 checks passed
@savannahostrowski savannahostrowski deleted the gh-122679 branch November 22, 2024 00:36
@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Nov 22, 2024
@miss-islington-app
Copy link

Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 22, 2024
@savannahostrowski @miss-islington
pythonGH-122679: Add register() to argparse docs (pythonGH-126939)
7692777 
* Add register() to argparse docs

* Add newline

* Formatting

* Fix codeblock

* Move section

* Add signature

* Add newline

* Fix indent

* Fix indent take 2

* Rephrase

* Simplify language

* Address PR comments

* Add references to register in type and action

* Remove unnecessary reference

* Rephrase and add success case
(cherry picked from commit fcfdb55)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 22, 2024
@savannahostrowski @miss-islington
pythonGH-122679: Add register() to argparse docs (pythonGH-126939)
77a4813 
* Add register() to argparse docs

* Add newline

* Formatting

* Fix codeblock

* Move section

* Add signature

* Add newline

* Fix indent

* Fix indent take 2

* Rephrase

* Simplify language

* Address PR comments

* Add references to register in type and action

* Remove unnecessary reference

* Rephrase and add success case
(cherry picked from commit fcfdb55)

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

bedevere-app bot commented Nov 22, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Nov 22, 2024
@bedevere-app
Copy link

bedevere-app bot commented Nov 22, 2024

GH-127149 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 Nov 22, 2024
@serhiy-storchaka
Copy link
Member

Documentation and test changes are usually backported. This will help to backport more important chages.

@serhiy-storchaka
Copy link
Member

And please edit commit message before merging. The history of intermediate changes is not interesting. The commit message should only reflect the final result.

serhiy-storchaka pushed a commit that referenced this pull request Nov 22, 2024
@miss-islington @savannahostrowski
[3.13] GH-122679: Add register() to argparse docs (GH-126939) (GH-1…
1b58c0f 
…27149)

(cherry picked from commit fcfdb55)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
serhiy-storchaka pushed a commit that referenced this pull request Nov 22, 2024
@miss-islington @savannahostrowski
[3.12] GH-122679: Add register() to argparse docs (GH-126939) (GH-1…
32bc8e8 
…27148)

(cherry picked from commit fcfdb55)

Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@savannahostrowski @ebonnal
pythonGH-122679: Add register() to argparse docs (python#126939)
520027b 
* Add register() to argparse docs

* Add newline

* Formatting

* Fix codeblock

* Move section

* Add signature

* Add newline

* Fix indent

* Fix indent take 2

* Rephrase

* Simplify language

* Address PR comments

* Add references to register in type and action

* Remove unnecessary reference

* Rephrase and add success case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielhollas danielhollas danielhollas left review comments

@picnixz picnixz picnixz left review comments

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
Docs PRs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

argparse: allow to override the type name in 'invalid %(type)s value: %(value)r' error messages
4 participants
@savannahostrowski @serhiy-storchaka @danielhollas @picnixz