Merge #12776,#11839: call argparse type function only once.

Before, the type function was called twice in the case where the default
was specified and the argument was given as well.  This was especially
problematic for the FileType type, as a default file would always be
opened, even if a file argument was specified on the command line.

Patch by Arnaud Fontaine, with additional test by Mike Meyer.
This commit is contained in:
R David Murray 2012-08-31 23:09:34 -04:00
commit 64b0ef1509
4 changed files with 74 additions and 7 deletions

View file

@ -1722,10 +1722,7 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer):
if action.dest is not SUPPRESS: if action.dest is not SUPPRESS:
if not hasattr(namespace, action.dest): if not hasattr(namespace, action.dest):
if action.default is not SUPPRESS: if action.default is not SUPPRESS:
default = action.default setattr(namespace, action.dest, action.default)
if isinstance(action.default, str):
default = self._get_value(action, default)
setattr(namespace, action.dest, default)
# add any parser defaults that aren't present # add any parser defaults that aren't present
for dest in self._defaults: for dest in self._defaults:
@ -1948,9 +1945,24 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer):
# if we didn't consume all the argument strings, there were extras # if we didn't consume all the argument strings, there were extras
extras.extend(arg_strings[stop_index:]) extras.extend(arg_strings[stop_index:])
# make sure all required actions were present # make sure all required actions were present and also convert
required_actions = [_get_action_name(action) for action in self._actions # action defaults which were not given as arguments
if action.required and action not in seen_actions] required_actions = []
for action in self._actions:
if action not in seen_actions:
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
hasattr(namespace, action.dest) and
action.default is getattr(namespace, action.dest)):
setattr(namespace, action.dest,
self._get_value(action, action.default))
if required_actions: if required_actions:
self.error(_('the following arguments are required: %s') % self.error(_('the following arguments are required: %s') %
', '.join(required_actions)) ', '.join(required_actions))

View file

@ -1463,6 +1463,22 @@ class TestFileTypeR(TempDirMixin, ParserTestCase):
('readonly', NS(x=None, spam=RFile('readonly'))), ('readonly', NS(x=None, spam=RFile('readonly'))),
] ]
class TestFileTypeDefaults(TempDirMixin, ParserTestCase):
"""Test that a file is not created unless the default is needed"""
def setUp(self):
super(TestFileTypeDefaults, self).setUp()
file = open(os.path.join(self.temp_dir, 'good'), 'w')
file.write('good')
file.close()
argument_signatures = [
Sig('-c', type=argparse.FileType('r'), default='no-file.txt'),
]
# should provoke no such file error
failures = ['']
# should not provoke error because default file is created
successes = [('-c good', NS(c=RFile('good')))]
class TestFileTypeRB(TempDirMixin, ParserTestCase): class TestFileTypeRB(TempDirMixin, ParserTestCase):
"""Test the FileType option/argument type for reading files""" """Test the FileType option/argument type for reading files"""
@ -4559,6 +4575,38 @@ class TestMessageContentError(TestCase):
self.assertNotIn(msg, 'optional_positional') self.assertNotIn(msg, 'optional_positional')
# ================================================
# Check that the type function is called only once
# ================================================
class TestTypeFunctionCallOnlyOnce(TestCase):
def test_type_function_call_only_once(self):
def spam(string_to_convert):
self.assertEqual(string_to_convert, 'spam!')
return 'foo_converted'
parser = argparse.ArgumentParser()
parser.add_argument('--foo', type=spam, default='bar')
args = parser.parse_args('--foo spam!'.split())
self.assertEqual(NS(foo='foo_converted'), args)
# ================================================================
# Check that the type function is called with a non-string default
# ================================================================
class TestTypeFunctionCallWithNonStringDefault(TestCase):
def test_type_function_call_with_non_string_default(self):
def spam(int_to_convert):
self.assertEqual(int_to_convert, 0)
return 'foo_converted'
parser = argparse.ArgumentParser()
parser.add_argument('--foo', type=spam, default=0)
args = parser.parse_args([])
self.assertEqual(NS(foo='foo_converted'), args)
# ====================== # ======================
# parse_known_args tests # parse_known_args tests
# ====================== # ======================

View file

@ -335,6 +335,7 @@ Nils Fischbeck
Frederik Fix Frederik Fix
Matt Fleming Matt Fleming
Hernán Martínez Foffani Hernán Martínez Foffani
Arnaud Fontaine
Michael Foord Michael Foord
Amaury Forgeot d'Arc Amaury Forgeot d'Arc
Doug Fort Doug Fort

View file

@ -16,6 +16,12 @@ Core and Builtins
Library Library
------- -------
- Issue #12776,#11839: call argparse type function (specified by add_argument)
only once. Before, the type function was called twice in the case where the
default was specified and the argument was given as well. This was
especially problematic for the FileType type, as a default file would always
be opened, even if a file argument was specified on the command line.
Extension Modules Extension Modules
----------------- -----------------