From 70369f4f2d1ef86e356d936d9f98e2ea23d8dec4 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Tue, 21 Dec 2021 16:28:46 +0800 Subject: [PATCH 1/3] Store defaults in namespace --- Lib/argparse.py | 43 +++++++++++++------ Lib/test/test_argparse.py | 31 +++++++------ .../2021-12-21-08-34-36.bpo-45235.OV9_9i.rst | 1 + 3 files changed, 49 insertions(+), 26 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index 2144c81886ad19..559e2cdfa90e8a 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1316,7 +1316,9 @@ def __call__(self, parser, namespace, values, option_string=None): # 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) + if key != '__defaults__': + setattr(namespace, key, value) + namespace.__defaults__.update(subnamespace.__defaults__) if arg_strings: if not hasattr(namespace, _UNRECOGNIZED_ARGS_ATTR): @@ -1398,6 +1400,9 @@ def __repr__(self): class Namespace(_AttributeHolder): """Simple object for storing attributes. + Default values are stored in a dict name `__defaults__` so they won't mess + with the given values. + Implements equality by attribute names and values, and provides a simple string representation. """ @@ -1405,14 +1410,26 @@ class Namespace(_AttributeHolder): def __init__(self, **kwargs): for name in kwargs: setattr(self, name, kwargs[name]) + self.__defaults__ = {} + + def _get_kwargs(self): + kwargs = self.__defaults__ | self.__dict__ + kwargs.pop('__defaults__', None) + return list(kwargs.items()) + + def __getattr__(self, name): + try: + return self.__defaults__[name] + except KeyError: + raise AttributeError(name) def __eq__(self, other): if not isinstance(other, Namespace): return NotImplemented - return vars(self) == vars(other) + return dict(self._get_kwargs()) == dict(other._get_kwargs()) def __contains__(self, key): - return key in self.__dict__ + return key in self.__dict__ or key in self.__defaults__ class _ActionsContainer(object): @@ -2025,14 +2042,13 @@ def _parse_known_args2(self, args, namespace, intermixed): # 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) + if action.default is not SUPPRESS and action.dest not in namespace: + namespace.__defaults__[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]) + if dest not in namespace: + namespace.__defaults__[dest] = self._defaults[dest] # parse the arguments and exit if there are any errors if self.exit_on_error: @@ -2326,12 +2342,11 @@ def consume_positionals(start_index): # 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)) + if (isinstance(action.default, str) and + action.dest in namespace and + getattr(namespace, action.dest) is action.default): + namespace.__defaults__[action.dest] = self._get_value( + action, action.default) 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..9c5756dca51304 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -121,18 +121,7 @@ def __init__(self, *args, **kwargs): self.kwargs = kwargs -class NS(object): - - def __init__(self, **kwargs): - self.__dict__.update(kwargs) - - def __repr__(self): - sorted_items = sorted(self.__dict__.items()) - kwarg_str = ', '.join(['%s=%r' % tup for tup in sorted_items]) - return '%s(%s)' % (type(self).__name__, kwarg_str) - - def __eq__(self, other): - return vars(self) == vars(other) +NS = argparse.Namespace class ArgumentParserError(Exception): @@ -2474,6 +2463,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() self.parser = self._get_parser() @@ -2940,6 +2939,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 # ============ diff --git a/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst b/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst new file mode 100644 index 00000000000000..4b7cb0c9ce27b6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst @@ -0,0 +1 @@ +Fix an issue that subparsers defaults override the existing values in the argparse Namespace. \ No newline at end of file From 4e106479993ddde433e55d549e02a47d5e0cd7a1 Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Tue, 21 Dec 2021 16:42:15 +0800 Subject: [PATCH 2/3] Reword the doc string --- Lib/argparse.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index 559e2cdfa90e8a..cd6dd6c650e9df 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1400,8 +1400,8 @@ def __repr__(self): class Namespace(_AttributeHolder): """Simple object for storing attributes. - Default values are stored in a dict name `__defaults__` so they won't mess - with the given values. + Default values are stored in a dict named `__defaults__` so they won't + override the given values. Implements equality by attribute names and values, and provides a simple string representation. From 96ace73b1d3f492bededc1df0ee1e86930bb6c4d Mon Sep 17 00:00:00 2001 From: Frost Ming Date: Thu, 1 Dec 2022 16:49:56 +0800 Subject: [PATCH 3/3] add newline to the end Signed-off-by: Frost Ming --- Lib/argparse.py | 28 ++++++++++++------- .../2021-12-21-08-34-36.bpo-45235.OV9_9i.rst | 2 +- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index cd6dd6c650e9df..3b0fbc5354f281 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1318,7 +1318,8 @@ def __call__(self, parser, namespace, values, option_string=None): for key, value in vars(subnamespace).items(): if key != '__defaults__': setattr(namespace, key, value) - namespace.__defaults__.update(subnamespace.__defaults__) + if hasattr(namespace, '__defaults__'): + namespace.__defaults__.update(subnamespace.__defaults__) if arg_strings: if not hasattr(namespace, _UNRECOGNIZED_ARGS_ATTR): @@ -1432,6 +1433,12 @@ def __contains__(self, key): return key in self.__dict__ or key in self.__defaults__ +def _set_default(namespace, dest, value): + if not hasattr(namespace, '__defaults__'): + setattr(namespace, dest, value) + else: + namespace.__defaults__[dest] = value + class _ActionsContainer(object): def __init__(self, @@ -2042,13 +2049,13 @@ def _parse_known_args2(self, args, namespace, intermixed): # add any action defaults that aren't present for action in self._actions: if action.dest is not SUPPRESS: - if action.default is not SUPPRESS and action.dest not in namespace: - namespace.__defaults__[action.dest] = action.default + if action.default is not SUPPRESS and not hasattr(namespace, action.dest): + _set_default(namespace, action.dest, action.default) # add any parser defaults that aren't present for dest in self._defaults: - if dest not in namespace: - namespace.__defaults__[dest] = self._defaults[dest] + if not hasattr(namespace, dest): + _set_default(namespace, dest, self._defaults[dest]) # parse the arguments and exit if there are any errors if self.exit_on_error: @@ -2342,11 +2349,12 @@ def consume_positionals(start_index): # 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 (isinstance(action.default, str) and - action.dest in namespace and - getattr(namespace, action.dest) is action.default): - namespace.__defaults__[action.dest] = self._get_value( - action, action.default) + if (action.default is not None and + isinstance(action.default, str) and + hasattr(namespace, action.dest) and + action.default is getattr(namespace, action.dest)): + _set_default(namespace, action.dest, + self._get_value(action, action.default)) if required_actions: raise ArgumentError(None, _('the following arguments are required: %s') % diff --git a/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst b/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst index 4b7cb0c9ce27b6..06c70cb3480948 100644 --- a/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst +++ b/Misc/NEWS.d/next/Library/2021-12-21-08-34-36.bpo-45235.OV9_9i.rst @@ -1 +1 @@ -Fix an issue that subparsers defaults override the existing values in the argparse Namespace. \ No newline at end of file +Fix an issue that subparsers defaults override the existing values in the argparse Namespace.