From 9ef5d46e9d248b47ee7ba0ce8116d7ea4a5e891c Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Tue, 16 Nov 2021 16:22:44 +0800 Subject: [PATCH 1/2] bpo-45235: Fix argparse namespace overriden by subparsers default --- Lib/argparse.py | 53 ++++++++++++++++----------------------- Lib/test/test_argparse.py | 42 +++++++++++++------------------ 2 files changed, 40 insertions(+), 55 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index 2144c81886ad19..2bb91fcbb01599 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1095,7 +1095,8 @@ def __init__(self, deprecated=deprecated) def __call__(self, parser, namespace, values, option_string=None): - items = getattr(namespace, self.dest, None) + items = getattr( + namespace, self.dest, self.default if self.default != SUPPRESS else None) items = _copy_items(items) items.append(values) setattr(namespace, self.dest, items) @@ -1124,7 +1125,8 @@ def __init__(self, deprecated=deprecated) def __call__(self, parser, namespace, values, option_string=None): - items = getattr(namespace, self.dest, None) + items = getattr( + namespace, self.dest, self.default if self.default != SUPPRESS else None) items = _copy_items(items) items.append(self.const) setattr(namespace, self.dest, items) @@ -1311,12 +1313,7 @@ def __call__(self, parser, namespace, values, option_string=None): # store any unrecognized options on the object, so that the top # level parser can decide what to do with them - # In case this subparser defines new defaults, we parse them - # in a new namespace object and then update the original - # namespace for the relevant parts. - subnamespace, arg_strings = subparser.parse_known_args(arg_strings, None) - for key, value in vars(subnamespace).items(): - setattr(namespace, key, value) + _, arg_strings = parser.parse_known_args(arg_strings, namespace) if arg_strings: if not hasattr(namespace, _UNRECOGNIZED_ARGS_ATTR): @@ -1325,7 +1322,8 @@ def __call__(self, parser, namespace, values, option_string=None): class _ExtendAction(_AppendAction): def __call__(self, parser, namespace, values, option_string=None): - items = getattr(namespace, self.dest, None) + items = getattr( + namespace, self.dest, self.default if self.default != SUPPRESS else None) items = _copy_items(items) items.extend(values) setattr(namespace, self.dest, items) @@ -2022,18 +2020,6 @@ def _parse_known_args2(self, args, namespace, intermixed): if namespace is None: namespace = Namespace() - # add any action defaults that aren't present - for action in self._actions: - if action.dest is not SUPPRESS: - if not hasattr(namespace, action.dest): - if action.default is not SUPPRESS: - setattr(namespace, action.dest, action.default) - - # add any parser defaults that aren't present - for dest in self._defaults: - if not hasattr(namespace, dest): - setattr(namespace, dest, self._defaults[dest]) - # parse the arguments and exit if there are any errors if self.exit_on_error: try: @@ -2043,6 +2029,11 @@ def _parse_known_args2(self, args, namespace, intermixed): else: namespace, args = self._parse_known_args(args, namespace, intermixed) + # add any parser defaults that aren't present + for dest in self._defaults: + if not hasattr(namespace, dest): + setattr(namespace, dest, self._defaults[dest]) + if hasattr(namespace, _UNRECOGNIZED_ARGS_ATTR): args.extend(getattr(namespace, _UNRECOGNIZED_ARGS_ATTR)) delattr(namespace, _UNRECOGNIZED_ARGS_ATTR) @@ -2322,16 +2313,16 @@ def consume_positionals(start_index): if action.required: required_actions.append(_get_action_name(action)) else: - # Convert action default now instead of doing it before - # parsing arguments to avoid calling convert functions - # twice (which may fail) if the argument was given, but - # only if it was defined already in the namespace - if (action.default is not None and - isinstance(action.default, str) and - hasattr(namespace, action.dest) and - action.default is getattr(namespace, action.dest)): - setattr(namespace, action.dest, - self._get_value(action, action.default)) + # Set the default value if the arg is not present after + # all arguments are parsed. This ensures that default values + # from subparsers override correctly + if (action.dest != SUPPRESS and + not hasattr(namespace, action.dest) and + action.default != SUPPRESS): + default_value = action.default + if isinstance(default_value, str): + default_value = self._get_value(action, action.default) + setattr(namespace, action.dest, default_value) if required_actions: raise ArgumentError(None, _('the following arguments are required: %s') % diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index fc73174d98cd6f..1bd988efd90d4d 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2164,17 +2164,6 @@ def __call__(self, parser, namespace, value, option_string=None): # check destination and option string assert self.dest == 'spam', 'dest: %s' % self.dest assert option_string == '-s', 'flag: %s' % option_string - # when option is before argument, badger=2, and when - # option is after argument, badger= - expected_ns = NS(spam=0.25) - if value in [0.125, 0.625]: - expected_ns.badger = 2 - elif value in [2.0]: - expected_ns.badger = 84 - else: - raise AssertionError('value: %s' % value) - assert expected_ns == namespace, ('expected %s, got %s' % - (expected_ns, namespace)) except AssertionError as e: raise ArgumentParserError('opt_action failed: %s' % e) setattr(namespace, 'spam', value) @@ -2187,19 +2176,6 @@ def __call__(self, parser, namespace, value, option_string=None): option_string) # check destination assert self.dest == 'badger', 'dest: %s' % self.dest - # when argument is before option, spam=0.25, and when - # option is after argument, spam= - expected_ns = NS(badger=2) - if value in [42, 84]: - expected_ns.spam = 0.25 - elif value in [1]: - expected_ns.spam = 0.625 - elif value in [2]: - expected_ns.spam = 0.125 - else: - raise AssertionError('value: %s' % value) - assert expected_ns == namespace, ('expected %s, got %s' % - (expected_ns, namespace)) except AssertionError as e: raise ArgumentParserError('arg_action failed: %s' % e) setattr(namespace, 'badger', value) @@ -2473,6 +2449,16 @@ def _get_parser(self, subparser_help=False, prefix_chars=None, # return the main parser return parser + + def _get_parser_with_shared_option(self): + parser = ErrorRaisingArgumentParser(prog='PROG', description='main description') + parser.add_argument('-f', '--foo', default='0') + subparsers = parser.add_subparsers() + parser1 = subparsers.add_parser('1') + parser1.add_argument('-f', '--foo', default='1') + parser2 = subparsers.add_parser('2') + parser2.add_argument('-f', '--foo', default='2') + return parser def setUp(self): super().setUp() @@ -2940,6 +2926,14 @@ def test_alias_help(self): 3 3 help """)) + def test_subparsers_with_shared_option(self): + parser = self._get_parser_with_shared_option() + self.assertEqual(parser.parse_args([]), NS(foo='0')) + self.assertEqual(parser.parse_args(['1']), NS(foo='1')) + self.assertEqual(parser.parse_args(['2']), NS(foo='2')) + self.assertEqual(parser.parse_args(['-f', '10', '1', '-f', '42']), NS(foo='42')) + self.assertEqual(parser.parse_args(['1'], NS(foo='42')), NS(foo='42')) + # ============ # Groups tests # ============ From 819b8804c2ca81f2236b2b5da0ad6c57706da2d9 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 16 Nov 2021 08:55:25 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Frost Ming --- Lib/test/test_argparse.py | 2 +- .../next/Library/2021-11-16-08-55-25.bpo-45235.OV9_9i.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2021-11-16-08-55-25.bpo-45235.OV9_9i.rst diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 1bd988efd90d4d..3232dc46ee3466 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2449,7 +2449,7 @@ def _get_parser(self, subparser_help=False, prefix_chars=None, # return the main parser return parser - + def _get_parser_with_shared_option(self): parser = ErrorRaisingArgumentParser(prog='PROG', description='main description') parser.add_argument('-f', '--foo', default='0') diff --git a/Misc/NEWS.d/next/Library/2021-11-16-08-55-25.bpo-45235.OV9_9i.rst b/Misc/NEWS.d/next/Library/2021-11-16-08-55-25.bpo-45235.OV9_9i.rst new file mode 100644 index 00000000000000..06c70cb3480948 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-11-16-08-55-25.bpo-45235.OV9_9i.rst @@ -0,0 +1 @@ +Fix an issue that subparsers defaults override the existing values in the argparse Namespace.