-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
gh-132558: allow choices
to be specified as strings in presence of a type
argument
#132743
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
base: main
Are you sure you want to change the base?
Conversation
This will allow better error messages.
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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.
You also need to...
- Update docs
- Add a NEWS entry
- Sign the CLA
Done, thanks for the help! |
Note that inclusion in the *choices* sequence is checked after any type_ | ||
conversions have been performed, so the type of the objects in the *choices* | ||
sequence should match the type_ specified. |
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.
So it was never a bug ;-)
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.
Okay, you got me. I'm convinced now :-)
:mod:`argparse` now allows ``choices`` to be entered as strings and calls | ||
convert on the available choices during checking. |
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.
Talk about the new option, keep it brief. Eg. Added X to :mod:... which allows ... .
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.
Hey there - thanks for jumping in on a PR. However, as mentioned on the issue, maybe we can start by clarifying the documentation. I’d prefer not to expand the API surface area by strapping another parameter onto ArgumentParser. This is a pattern we need to be really intentional about.
As a motivating example, consider the following program:
This gives the following usage string:
With the proposed pull request, the program can be rewritten as follows:
With the following usage string:
I took care to ensure backward compatibility including by adding a feature flag for the behavior. The feature flag should not be necessary, but because the argparse library is so flexible, it better to be safe than sorry.
choices
withtype
#132558