classification
Title: argparse: bad nargs value raises misleading message
Type: behavior Stage: needs patch
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 3.4, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Robert, bethard, chris.jerdonek, paul.j3
Priority: normal Keywords: easy, patch

Created on 2013-01-15 12:09 by chris.jerdonek, last changed 2013-04-25 16:54 by paul.j3.

Files
File name Uploaded Description Edit
argparse.patch Robert, 2013-01-17 20:41 review
argparse-v2.patch Robert, 2013-01-18 14:58 review
Messages (8)
msg180012 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-15 12:09
>>> parser = argparse.ArgumentParser()
>>> parser.add_argument('foo', nargs='a')
  ...
  File ".../Lib/argparse.py", line 1333, in add_argument
    raise ValueError("length of metavar tuple does not match nargs")
ValueError: length of metavar tuple does not match nargs

The message should really be about nargs not having a valid value.  The nargs value is invalid regardless of the metavar.  There is also this:

>>> 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)

which is not consistent with this:

>>> parser.add_argument('foo', nargs=0)
  ...
    raise ValueError('nargs for store actions must be > 0; if you '
ValueError: nargs for store actions must be > 0; if you have nothing to store, actions such as store true or store const may be more appropriate
msg180152 - (view) Author: Robert Leenders (Robert) Date: 2013-01-17 20:41
Attached is a patch which solves these problems and adds test cases of Chris Jerdonek. When nargs is negative the same ValueError is raised as when nargs is zero. Also when nargs is any other invalid value a ValueError("invalid value for nargs") is raised.

I have one question and that is about the value PARSER="A..." for nargs, it's a valid option and there are test cases with nargs="A...", however it is not listed in the documentation: http://docs.python.org/dev/library/argparse.html#nargs
msg180154 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-17 21:11
Thanks for the patch.  I will take a look.  And good observation re: PARSER.  Can you open a separate documentation issue for that where it can be discussed?
msg180156 - (view) Author: Robert Leenders (Robert) Date: 2013-01-17 21:33
The new issue about PARSER can be found here: http://bugs.python.org/issue16988
msg180180 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013-01-18 09:08
I added some Rietveld comments.
msg180194 - (view) Author: Robert Leenders (Robert) Date: 2013-01-18 14:48
Chris, you said (in the review) "Hmm, since this is for maintenance releases ... This change could cause "working" code to no longer work."

I understood from our original message that you wanted it to change since it is inconsistent. I vote for changing it (so that it gives an error) but since this is my first bug/patch, I don't really know what usually happens.

Either way, I adjusted the patch conform your comments. For now I removed the original changes to handle negative numbers and changed the message from "nargs must be > 0" to "nargs must be != 0".
msg187755 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-04-25 04:51
http://bugs.python.org/issue9849
also deals with nargs values.  However there the focus is on string values like '1', not whether an integer value can be 0 or <0.

I submitted a patch that moves the nargs testing to a ArgumentParser._check_argument() method.  It still depends on _format_args to do the actual testing of nargs.  I also make sure that group.add_argument() does this test.

Regarding this issue, can nargs=0 or <0?  _Store_action objects to 0, not because it causes runtime errors, but because it does not make sense.  Code with nargs=-1 runs without error, not consuming a string and returning [], just as a nargs=0 would.  

In http://bugs.python.org/issue14191 I found it useful to temporarily set nargs=0 to 'turn off' a positional.

I would vote for leaving this error message as is:

"ValueError: nargs for store actions must be > 0; if you have nothing to store, actions such as store true or store const may be more appropriate"

even though the test is actually nargs==0.  For normal use the recommendation that nargs>0 makes sense.
msg187792 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-04-25 16:54
An integer nargs value is only used in one of 2 ways,

range(nargs)

'%s'*nargs

In both a negative value acts the same as a 0.

I don't think the original authors though much about 'what if the code user gives a negative value?', because nargs is counting things - the number of expected arguments.  For some actions that number is 0.  For other some sort of positive integer, or variable numbers like '*','+' make most sense.

To some degree nargs is modeled on the regex sequences, '*','+','?','{n}'.  '{-1}' does not produce a regex error, though I can't make anything match it.
History
Date User Action Args
2013-04-25 16:54:09paul.j3setmessages: + msg187792
2013-04-25 04:51:28paul.j3setnosy: + paul.j3
messages: + msg187755
2013-01-18 14:58:15Robertsetfiles: + argparse-v2.patch
2013-01-18 14:56:54Robertsetfiles: - argparse-v2.patch
2013-01-18 14:48:19Robertsetfiles: + argparse-v2.patch

messages: + msg180194
2013-01-18 09:08:25chris.jerdoneksetmessages: + msg180180
2013-01-17 21:33:08Robertsetmessages: + msg180156
2013-01-17 21:11:04chris.jerdoneksetmessages: + msg180154
2013-01-17 20:41:15Robertsetfiles: + argparse.patch

nosy: + Robert
messages: + msg180152

keywords: + patch
2013-01-15 12:09:51chris.jerdonekcreate