Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -1316,7 +1316,10 @@ 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)
if hasattr(namespace, '__defaults__'):
namespace.__defaults__.update(subnamespace.__defaults__)

if arg_strings:
if not hasattr(namespace, _UNRECOGNIZED_ARGS_ATTR):
Expand Down Expand Up @@ -1398,23 +1401,44 @@ def __repr__(self):
class Namespace(_AttributeHolder):
"""Simple object for storing attributes.

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

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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you convert it into a list if the caller (__eq__) turns it into a dict again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __repr__ implementation of the parent class _AttributeHolder also calls this. But this change can be considered.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I should have checked that. Better to just leave it as is then.


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__


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,
Expand Down Expand Up @@ -2025,14 +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 not hasattr(namespace, action.dest):
if action.default is not SUPPRESS:
setattr(namespace, 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 not hasattr(namespace, dest):
setattr(namespace, dest, self._defaults[dest])
_set_default(namespace, dest, self._defaults[dest])

# parse the arguments and exit if there are any errors
if self.exit_on_error:
Expand Down Expand Up @@ -2330,7 +2353,7 @@ def consume_positionals(start_index):
isinstance(action.default, str) and
hasattr(namespace, action.dest) and
action.default is getattr(namespace, action.dest)):
setattr(namespace, action.dest,
_set_default(namespace, action.dest,
self._get_value(action, action.default))

if required_actions:
Expand Down
31 changes: 19 additions & 12 deletions Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
# ============
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix an issue that subparsers defaults override the existing values in the argparse Namespace.
Loading