Argparse needs better error handling for nargs

[JasonBaker]

BPO	9849
Nosy	@rhettinger, @merwok, @iritkatriel
Files	nargswarn.patch test_nargswarn.py nargswarn.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/rhettinger'
closed_at = <Date 2020-11-07.00:37:17.204>
created_at = <Date 2010-09-13.23:25:11.224>
labels = ['library']
title = 'Argparse needs better error handling for nargs'
updated_at = <Date 2020-11-07.00:37:17.202>
user = 'https://bugs.python.org/JasonBaker'

bugs.python.org fields:

activity = <Date 2020-11-07.00:37:17.202>
actor = 'iritkatriel'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2020-11-07.00:37:17.204>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2010-09-13.23:25:11.224>
creator = 'Jason.Baker'
dependencies = []
files = ['29998', '29999', '30010']
hgrepos = []
issue_num = 9849
keywords = ['patch']
message_count = 12.0
messages = ['116355', '116357', '186962', '187132', '187166', '187614', '187686', '187687', '187754', '192719', '220017', '380484']
nosy_count = 6.0
nosy_names = ['rhettinger', 'bethard', 'eric.araujo', 'Jason.Baker', 'paul.j3', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue9849'
versions = ['Python 2.7', 'Python 3.2']

[JasonBaker]

This is referring to argparse 1.1 installed under Python 2.6. When I was passing in an nargs flag, I figured that since '+' and '*' are valid options, I should pass in strings. So when I tried passing in the string '1' instead of the integer 1, I got the following error:

>> import argparse

>>> parser = argparse.ArgumentParser()
>>> parser.add_argument('foo', nargs='1')
_StoreAction(option_strings=[], dest='foo', nargs='1', const=None, default=None, type=None, choices=None, help=None, metavar=None)

>>> parser.parse_args()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "build/bdist.macosx-10.6-universal/egg/argparse.py", line 1698, in parse_args
  File "build/bdist.macosx-10.6-universal/egg/argparse.py", line 1730, in parse_known_args
  File "build/bdist.macosx-10.6-universal/egg/argparse.py", line 1935, in _parse_known_args
  File "build/bdist.macosx-10.6-universal/egg/argparse.py", line 1884, in consume_positionals
  File "build/bdist.macosx-10.6-universal/egg/argparse.py", line 2028, in _match_arguments_partial
  File "build/bdist.macosx-10.6-universal/egg/argparse.py", line 2169, in _get_nargs_pattern
TypeError: can't multiply sequence by non-int of type 'str'

Fortunately, I had just added the nargs and knew to correct that. However, if I were to do something like read that value in from a config file and forget to coerce the value from a string to an int, I could see how this could be a giant pain to track down.

[ned-deily]

Note, argparse is not part of the Python standard library in 2.6 but the 2.7 and 3.2 versions exhibit the same behavior.

[BreamoreBoy]

The behaviour has changed from that given in msg116355. Using Python 3.3.1 on Windows.

>>> import argparse
>>> parser = argparse.ArgumentParser()
>>> parser.add_argument('foo', nargs='1')
Traceback (most recent call last):
  File "c:\python33\lib\argparse.py", line 1322, in add_argument
    self._get_formatter()._format_args(action, None)
  File "c:\python33\lib\argparse.py", line 585, in _format_args
    formats = ['%s' for _ in range(action.nargs)]
TypeError: 'str' object cannot be interpreted as an integer

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\python33\lib\argparse.py", line 1324, in add_argument
    raise ValueError("length of metavar tuple does not match nargs")
ValueError: length of metavar tuple does not match nargs

The docs http://docs.python.org/3/library/argparse.html#nargs are clear that nargs is an integer or various types of string so I think this could be closed as already fixed.

[paulj3]

It does shift the error from parse_args to add_argument, but the message 'ValueError: length of metavar tuple does not match nargs', indicates that it's a side effect of checking on the tuple form of metavar.
http://bugs.python.org/issue9348

There is still room for cleaning up these tests. There are 2 functions that define what are acceptable values for nargs, [None, OPTIONAL, ZERO_OR_MORE, ONE_OR_MORE, REMAINDER, PARSER, integer]. Should one or other explicitly check nargs is an integer if it does not match one of the other strings?

And the test in _ActionContainer.add_argument()

    # raise an error if the metavar does not match the type
    if hasattr(self, "_get_formatter"):
        try:
            self._get_formatter()._format_args(action, None)
        except TypeError:
            raise ValueError("length of metavar tuple does not match nargs")

uses an ArgumentParser._get_formatter method (though 'hasattr' prevents runtime errors). In fact both functions that use nargs belong to the parser, not the container. I wonder if add_argument should be moved to ArgumentParser.

[BreamoreBoy]

The first error raised is "TypeError: 'str' object cannot be interpreted as an integer", followed by "ValueError: length of metavar tuple does not match nargs". Therefore the code has already been changed to reflect the title of this issue. If other code changes are needed I believe that should be done in a new or another already existing issue.

[paulj3]

This nargs test using the formater applies only when the container has a help formatter. That is true for a ArgumentParser, but not for an argument_group.

group = parser.add_argument_group('g')
group.add_argument('bar', nargs='test')

does not raise an error.

format_help will produce an error: ...
File "./argparse.py", line 585, in _format_args
formats = ['%s' for _ in range(action.nargs)]
TypeError: 'str' object cannot be interpreted as an integer

while parse_args produces the error: ...
      File "./argparse.py", line 2200, in _get_nargs_pattern
        nargs_pattern = '(-*%s-*)' % '-*'.join('A' * nargs)
    TypeError: can't multiply sequence by non-int of type 'str'

[paulj3]

This patch adds a value test for nargs during add_argument. The core of the new test is in ArgumentParser._check_argument.

add_argument() now calls ArgumentParser._check_argument(), which calls _format_args(). If it gets a TypeError, it raises a metavar ValueError as before. But if it gets an ValueError, it raises an ArgumentError.

An argument group gets the _check_argument method from its container. This way, check_argument() works for both parsers and groups.

HelpFormater _format_args() now raises a ValueError if the nargs is not an integer (or one of the recognized strings).

What kind of error should we produce when there is a problem with nargs?

An ArgumentError has the advantage that it includes the action name. But the metavar tuple test was coded to raise ValueError. Plus the test_argparse.py TestInvalidArgumentConstructors class tests all check for TypeError or ValueError.

I have kept the metavar tuple case as ValueError. That way, test_argparse.py runs without a change. I still need to add tests for invalid string nargs values. And I think the case could be made for returning ArgumentValue errors.

[paulj3]

The attached test_nargswarn.py file tests the argparse.py patch. It tries out various nargs values, both for parsers, groups, and mutually exclusive groups. I intend to recast these to work in test_argparse.py

[paulj3]

This is a revision of yesterday's patch. It includes a couple of test cases that check that parser, groups, and exclusive groups all produce the error message.

I also changed the metavar tuple case to return an ArgumentError, and changed test_argparse.py accordingly. (this metavar testing was added in http://bugs.python.org/issue9348).

This issue overlaps with http://bugs.python.org/issue16970, 'argparse: bad nargs value raises misleading message'. There, though, the focus is more on the error message when nargs=0. While nargs<1 might not make much sense, it technically does not break anything.

[paulj3]

I included this patch (with minor changes) in a patch that I just posted to http://bugs.python.org/issue16468. That issue deals with the argument choices option, which can be tested along with nargs and metavars.

[paulj3]

http://bugs.python.org/issue21666
raises the possibility of testing the 'help' parameter in the same way. By adding (to _check_argument):

    # check the 'help' string
    try:
        self._get_formatter()._expand_help(action)
    except (ValueError, TypeError, KeyError) as e:
        raise ArgumentError(action, 'badly formed help string')