[Python-checkins] cpython (merge 3.2 -> default): Merge #12776,#11839: call argparse type function only once.

r.david.murray python-checkins at python.org
Sat Sep 1 05:16:51 CEST 2012


http://hg.python.org/cpython/rev/74f6d87cd471
changeset:   78822:74f6d87cd471
parent:      78819:815b88454e3e
parent:      78821:1b614921aefa
user:        R David Murray <rdmurray at bitdance.com>
date:        Fri Aug 31 23:09:34 2012 -0400
summary:
  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.

files:
  Lib/argparse.py           |  26 ++++++++++---
  Lib/test/test_argparse.py |  48 +++++++++++++++++++++++++++
  Misc/ACKS                 |   1 +
  Misc/NEWS                 |   6 +++
  4 files changed, 74 insertions(+), 7 deletions(-)


diff --git a/Lib/argparse.py b/Lib/argparse.py
--- a/Lib/argparse.py
+++ b/Lib/argparse.py
@@ -1722,10 +1722,7 @@
             if action.dest is not SUPPRESS:
                 if not hasattr(namespace, action.dest):
                     if action.default is not SUPPRESS:
-                        default = action.default
-                        if isinstance(action.default, str):
-                            default = self._get_value(action, default)
-                        setattr(namespace, action.dest, default)
+                        setattr(namespace, action.dest, action.default)
 
         # add any parser defaults that aren't present
         for dest in self._defaults:
@@ -1948,9 +1945,24 @@
         # if we didn't consume all the argument strings, there were extras
         extras.extend(arg_strings[stop_index:])
 
-        # make sure all required actions were present
-        required_actions = [_get_action_name(action) for action in self._actions
-                            if action.required and action not in seen_actions]
+        # make sure all required actions were present and also convert
+        # action defaults which were not given as arguments
+        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:
             self.error(_('the following arguments are required: %s') %
                        ', '.join(required_actions))
diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py
--- a/Lib/test/test_argparse.py
+++ b/Lib/test/test_argparse.py
@@ -1463,6 +1463,22 @@
         ('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):
     """Test the FileType option/argument type for reading files"""
@@ -4559,6 +4575,38 @@
         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
 # ======================
diff --git a/Misc/ACKS b/Misc/ACKS
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -335,6 +335,7 @@
 Frederik Fix
 Matt Fleming
 Hernán Martínez Foffani
+Arnaud Fontaine
 Michael Foord
 Amaury Forgeot d'Arc
 Doug Fort
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -16,6 +16,12 @@
 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
 -----------------
 

-- 
Repository URL: http://hg.python.org/cpython


More information about the Python-checkins mailing list