Skip to content

Commit

Permalink
argparse: use str() consistently and explicitly to print choices
Browse files Browse the repository at this point in the history
Fixes: #86357

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
  • Loading branch information
rindeal committed Sep 28, 2024
1 parent c58c572 commit 549391c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 10 deletions.
12 changes: 5 additions & 7 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,7 @@ def _metavar_formatter(self, action, default_metavar):
if action.metavar is not None:
result = action.metavar
elif action.choices is not None:
choice_strs = [str(choice) for choice in action.choices]
result = '{%s}' % ','.join(choice_strs)
result = '{%s}' % ','.join(map(str, action.choices))
else:
result = default_metavar

Expand Down Expand Up @@ -597,8 +596,7 @@ def _expand_help(self, action):
if hasattr(params[name], '__name__'):
params[name] = params[name].__name__
if params.get('choices') is not None:
choices_str = ', '.join([str(c) for c in params['choices']])
params['choices'] = choices_str
params['choices'] = ', '.join(map(str, params['choices']))
return self._get_help_string(action) % params

def _iter_indented_subactions(self, action):
Expand Down Expand Up @@ -707,7 +705,7 @@ def _get_action_name(argument):
elif argument.dest not in (None, SUPPRESS):
return argument.dest
elif argument.choices:
return '{' + ','.join(argument.choices) + '}'
return '{%s}' % ','.join(map(str, argument.choices))
else:
return None

Expand Down Expand Up @@ -2556,8 +2554,8 @@ def _check_value(self, action, value):
# converted value must be one of the choices (if specified)
if action.choices is not None and value not in action.choices:
args = {'value': value,
'choices': ', '.join(map(repr, action.choices))}
msg = _('invalid choice: %(value)r (choose from %(choices)s)')
'choices': ', '.join(map(str, action.choices))}
msg = _('invalid choice: %(value)s (choose from %(choices)s)')
raise ArgumentError(action, msg % args)

# =======================
Expand Down
36 changes: 33 additions & 3 deletions Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import argparse
import warnings

from enum import StrEnum
from test.support import os_helper, captured_stderr
from unittest import mock

Expand Down Expand Up @@ -952,6 +953,35 @@ class TestDisallowLongAbbreviationAllowsShortGroupingPrefix(ParserTestCase):
]


class TestStrEnumChoices(TestCase):
class Color(StrEnum):
RED = "red"
GREEN = "green"
BLUE = "blue"

def test_metavar_formatter_with_strenum(self):
parser = argparse.ArgumentParser()
parser.add_argument('--color', choices=self.Color)
args = parser.parse_args(['--color', 'red'])
self.assertEqual(args.color, self.Color.RED)

def test_expand_help_with_strenum(self):
parser = argparse.ArgumentParser()
parser.add_argument('--color', choices=self.Color, help='Choose a color')
help_output = parser.format_help()
self.assertIn('[--color {red,green,blue}]', help_output)
self.assertIn(' --color {red,green,blue}', help_output)

def test_check_value_with_strenum(self):
parser = argparse.ArgumentParser()
parser.add_argument('--color', choices=self.Color)
self.assertRaisesRegex(
argparse.ArgumentError,
r"invalid choice: yellow \(choose from red, green, blue\)",
parser.parse_args,
['--color', 'yellow'],
)

# ================
# Positional tests
# ================
Expand Down Expand Up @@ -2399,7 +2429,7 @@ def test_wrong_argument_subparsers_no_destination_error(self):
parser.parse_args(('baz',))
self.assertRegex(
excinfo.exception.stderr,
r"error: argument {foo,bar}: invalid choice: 'baz' \(choose from 'foo', 'bar'\)\n$"
r"error: argument {foo,bar}: invalid choice: baz \(choose from foo, bar\)\n$"
)

def test_optional_subparsers(self):
Expand Down Expand Up @@ -6061,7 +6091,7 @@ def test_subparser(self):
args = parser.parse_args(['x', '--', 'run', '--', 'a', '--', 'b'])
self.assertEqual(NS(foo='x', f=None, bar=['a', '--', 'b']), args)
self.assertRaisesRegex(argparse.ArgumentError,
"invalid choice: '--'",
"invalid choice: --",
parser.parse_args, ['--', 'x', '--', 'run', 'a', 'b'])

def test_subparser_after_multiple_argument_option(self):
Expand All @@ -6075,7 +6105,7 @@ def test_subparser_after_multiple_argument_option(self):
args = parser.parse_args(['--foo', 'x', 'y', '--', 'run', 'a', 'b', '-f', 'c'])
self.assertEqual(NS(foo=['x', 'y'], f='c', bar=['a', 'b']), args)
self.assertRaisesRegex(argparse.ArgumentError,
"invalid choice: '--'",
"invalid choice: --",
parser.parse_args, ['--foo', 'x', '--', '--', 'run', 'a', 'b'])


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Always use :func:`str` to print ``choices`` in :mod:`argparse`.

0 comments on commit 549391c

Please sign in to comment.