From a184a52982c97a4e4d73075a848518febfff8431 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 21 Sep 2024 22:32:42 +0300 Subject: [PATCH 1/2] gh-72795: Make positional arguments with nargs='*' or REMAINDER non-required This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments. --- Lib/argparse.py | 10 ++--- Lib/test/test_argparse.py | 45 ++++++++++++++----- ...4-09-21-22-32-21.gh-issue-72795.naLmkX.rst | 4 ++ 3 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-09-21-22-32-21.gh-issue-72795.naLmkX.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index 7988c447d03584..68dacc4fad12c4 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1533,9 +1533,8 @@ def _get_positional_kwargs(self, dest, **kwargs): # mark positional arguments as required if at least one is # always required - if kwargs.get('nargs') not in [OPTIONAL, ZERO_OR_MORE]: - kwargs['required'] = True - if kwargs.get('nargs') == ZERO_OR_MORE and 'default' not in kwargs: + nargs = kwargs.get('nargs') + if nargs not in [OPTIONAL, ZERO_OR_MORE, REMAINDER, SUPPRESS, 0]: kwargs['required'] = True # return the keyword arguments with no option strings @@ -1950,9 +1949,8 @@ def take_action(action, argument_strings, option_string=None): argument_values = self._get_values(action, argument_strings) # error if this argument is not allowed with other previously - # seen arguments, assuming that actions that use the default - # value don't really count as "present" - if argument_values is not action.default: + # seen arguments + if action.option_strings or argument_strings: seen_non_default_actions.add(action) for conflict_action in action_conflicts.get(action, []): if conflict_action in seen_non_default_actions: diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 138ff19e86acf4..b6174a61652b4b 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2877,26 +2877,30 @@ def test_failures_when_not_required(self): parse_args = self.get_parser(required=False).parse_args error = ArgumentParserError for args_string in self.failures: - self.assertRaises(error, parse_args, args_string.split()) + with self.subTest(args=args_string): + self.assertRaises(error, parse_args, args_string.split()) def test_failures_when_required(self): parse_args = self.get_parser(required=True).parse_args error = ArgumentParserError for args_string in self.failures + ['']: - self.assertRaises(error, parse_args, args_string.split()) + with self.subTest(args=args_string): + self.assertRaises(error, parse_args, args_string.split()) def test_successes_when_not_required(self): parse_args = self.get_parser(required=False).parse_args successes = self.successes + self.successes_when_not_required for args_string, expected_ns in successes: - actual_ns = parse_args(args_string.split()) - self.assertEqual(actual_ns, expected_ns) + with self.subTest(args=args_string): + actual_ns = parse_args(args_string.split()) + self.assertEqual(actual_ns, expected_ns) def test_successes_when_required(self): parse_args = self.get_parser(required=True).parse_args for args_string, expected_ns in self.successes: - actual_ns = parse_args(args_string.split()) - self.assertEqual(actual_ns, expected_ns) + with self.subTest(args=args_string): + actual_ns = parse_args(args_string.split()) + self.assertEqual(actual_ns, expected_ns) def test_usage_when_not_required(self): format_usage = self.get_parser(required=False).format_usage @@ -3073,7 +3077,7 @@ def get_parser(self, required): group = parser.add_mutually_exclusive_group(required=required) group.add_argument('--foo', action='store_true', help='FOO') group.add_argument('--spam', help='SPAM') - group.add_argument('badger', nargs='*', default='X', help='BADGER') + group.add_argument('badger', nargs='*', help='BADGER') return parser failures = [ @@ -3084,13 +3088,13 @@ def get_parser(self, required): '--foo X Y', ] successes = [ - ('--foo', NS(foo=True, spam=None, badger='X')), - ('--spam S', NS(foo=False, spam='S', badger='X')), + ('--foo', NS(foo=True, spam=None, badger=[])), + ('--spam S', NS(foo=False, spam='S', badger=[])), ('X', NS(foo=False, spam=None, badger=['X'])), ('X Y Z', NS(foo=False, spam=None, badger=['X', 'Y', 'Z'])), ] successes_when_not_required = [ - ('', NS(foo=False, spam=None, badger='X')), + ('', NS(foo=False, spam=None, badger=[])), ] usage_when_not_required = '''\ @@ -6207,6 +6211,27 @@ def test_required_args(self): 'the following arguments are required: bar, baz', self.parser.parse_args, []) + def test_required_args_optional(self): + self.parser.add_argument('bar') + self.parser.add_argument('baz', nargs='?') + self.assertRaisesRegex(argparse.ArgumentError, + 'the following arguments are required: bar', + self.parser.parse_args, []) + + def test_required_args_zero_or_more(self): + self.parser.add_argument('bar') + self.parser.add_argument('baz', nargs='*') + self.assertRaisesRegex(argparse.ArgumentError, + 'the following arguments are required: bar', + self.parser.parse_args, []) + + def test_required_args_remainder(self): + self.parser.add_argument('bar') + self.parser.add_argument('baz', nargs='...') + self.assertRaisesRegex(argparse.ArgumentError, + 'the following arguments are required: bar', + self.parser.parse_args, []) + def test_required_mutually_exclusive_args(self): group = self.parser.add_mutually_exclusive_group(required=True) group.add_argument('--bar') diff --git a/Misc/NEWS.d/next/Library/2024-09-21-22-32-21.gh-issue-72795.naLmkX.rst b/Misc/NEWS.d/next/Library/2024-09-21-22-32-21.gh-issue-72795.naLmkX.rst new file mode 100644 index 00000000000000..15c0918097367f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-09-21-22-32-21.gh-issue-72795.naLmkX.rst @@ -0,0 +1,4 @@ +Positional arguments with :ref:`nargs` equal to ``'*'`` or +:data:`!argparse.REMAINDER` are no longer required. This allows to use +positional argument with ``nargs='*'`` and without ``default`` in mutually +exclusive group and improves error message about required arguments. From a44612d3bcf21434031175053ff75a2c49cfb1c9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 24 Sep 2024 10:29:57 +0300 Subject: [PATCH 2/2] Make tests more strict. --- Lib/test/test_argparse.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index adb2a6b7b2f82f..a94c48a8dd822f 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -6355,28 +6355,28 @@ def test_required_args(self): self.parser.add_argument('bar') self.parser.add_argument('baz') self.assertRaisesRegex(argparse.ArgumentError, - 'the following arguments are required: bar, baz', + 'the following arguments are required: bar, baz$', self.parser.parse_args, []) def test_required_args_optional(self): self.parser.add_argument('bar') self.parser.add_argument('baz', nargs='?') self.assertRaisesRegex(argparse.ArgumentError, - 'the following arguments are required: bar', + 'the following arguments are required: bar$', self.parser.parse_args, []) def test_required_args_zero_or_more(self): self.parser.add_argument('bar') self.parser.add_argument('baz', nargs='*') self.assertRaisesRegex(argparse.ArgumentError, - 'the following arguments are required: bar', + 'the following arguments are required: bar$', self.parser.parse_args, []) def test_required_args_remainder(self): self.parser.add_argument('bar') self.parser.add_argument('baz', nargs='...') self.assertRaisesRegex(argparse.ArgumentError, - 'the following arguments are required: bar', + 'the following arguments are required: bar$', self.parser.parse_args, []) def test_required_mutually_exclusive_args(self):