Skip to content

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hansthen
Copy link

@hansthen hansthen commented Apr 20, 2025

As a motivating example, consider the following program:

import argparse

parser = argparse.ArgumentParser()

day_names = ["mo", "tu", "we", "th", "fr", "sa", "su"]
days = [1, 2, 3, 4, 5, 6, 7]

def name_day(value):
    return day_names.index(value) + 1

parser.add_argument(
    '--day',
    type=name_day,
    choices=days,
)
parser.parse_args()

This gives the following usage string:

usage: example.py [-h] [--day {1,2,3,4,5,6,7}]

With the proposed pull request, the program can be rewritten as follows:

import argparse

parser = argparse.ArgumentParser()

day_names = ["mo", "tu", "we", "th", "fr", "sa", "su"]

def name_day(value):
    return day_names.index(value) + 1

parser.add_argument(
    '--day',
    type=name_day,
    choices=days,
)
parser.parse_args()

With the following usage string:

usage: example_new.py [-h] [--days {mo,tu,we,th,fr,sa,su}]

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.

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 20, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Apr 20, 2025

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 skip news label instead.

Copy link
Contributor

@StanFromIreland StanFromIreland left a 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

@hansthen
Copy link
Author

You also need to...

  • Update docs
  • Add a NEWS entry
  • Sign the CLA

Done, thanks for the help!

Comment on lines -1127 to -1129
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.
Copy link
Contributor

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 ;-)

Copy link
Author

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 :-)

Comment on lines 1 to 2
:mod:`argparse` now allows ``choices`` to be entered as strings and calls
convert on the available choices during checking.
Copy link
Contributor

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 ... .

Copy link
Member

@savannahostrowski savannahostrowski left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants