Re: [Python-Dev] [Python-checkins] cpython (merge 3.2 -> default): - Issue #15906: Fix a regression in argparse caused by the preceding change,
On Tue, Sep 11, 2012 at 9:13 PM, barry.warsaw <python-checkins@python.org> wrote:
http://hg.python.org/cpython/rev/25e41fdc4e60 changeset: 79001:25e41fdc4e60 parent: 78998:6fea947edead parent: 79000:bc342cd7ed96 user: Barry Warsaw <barry@python.org> date: Wed Sep 12 00:12:29 2012 -0400 summary: - Issue #15906: Fix a regression in argparse caused by the preceding change, when action='append', type='str' and default=[].
I didn't have time to respond Barry's e-mail from four hours ago before this was committed. I think this change may be problematic. At the least, I think people should have an opportunity to air their specific concerns and talk through the implications. Also, from the discussion it seemed like the sentiment was leaning towards a different approach for the fix. I made a comment on the issue with some more extended remarks: http://bugs.python.org/msg170351 --Chris
files: Lib/argparse.py | 1 + Lib/test/test_argparse.py | 10 ++++++++++ Misc/NEWS | 3 +++ 3 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/Lib/argparse.py b/Lib/argparse.py --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1961,6 +1961,7 @@ # 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, _StoreAction) and hasattr(namespace, action.dest) and action.default is getattr(namespace, action.dest)): setattr(namespace, action.dest, 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 @@ -4607,6 +4607,16 @@ args = parser.parse_args([]) self.assertEqual(NS(foo='foo_converted'), args)
+ def test_issue_15906(self): + # Issue #15906: When action='append', type=str, default=[] are + # providing, the dest value was the string representation "[]" when it + # should have been an empty list. + parser = argparse.ArgumentParser() + parser.add_argument('--test', dest='test', type=str, + default=[], action='append') + args = parser.parse_args([]) + self.assertEqual(args.test, []) + # ====================== # parse_known_args tests # ====================== diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -56,6 +56,9 @@ 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.
+- Issue #15906: Fix a regression in argparse caused by the preceding change, + when action='append', type='str' and default=[]. + Extension Modules -----------------
-- Repository URL: http://hg.python.org/cpython
_______________________________________________ Python-checkins mailing list Python-checkins@python.org http://mail.python.org/mailman/listinfo/python-checkins
On Sep 11, 2012, at 09:30 PM, Chris Jerdonek wrote:
I didn't have time to respond Barry's e-mail from four hours ago before this was committed. I think this change may be problematic. At the least, I think people should have an opportunity to air their specific concerns and talk through the implications.
Also, from the discussion it seemed like the sentiment was leaning towards a different approach for the fix.
I made a comment on the issue with some more extended remarks:
The alternative suggested fix breaks the test suite (yes I tried it). So maybe we have to go back and decide whether the original fix for #12776 and #11839 is correct. More detail in the tracker issue. -Barry
participants (2)
-
Barry Warsaw -
Chris Jerdonek