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

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

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?

The new issue about PARSER can be found here: http://bugs.python.org/issue16988

I added some Rietveld comments.

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".

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.

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.

Hello!

I added the patch and submitted the PR and ran the test, could you please take a look?

Also, I see this 3.6

Thanks

New changeset 4b3e97592376d5f8a3b75192b399a2da1be642cb by Miss Islington (bot) (tmblweed) in branch 'master':
bpo-16970: Adding error message for invalid args (GH-14844)
https://github.com/python/cpython/commit/4b3e97592376d5f8a3b75192b399a2da1be642cb

The 3.8 backport will land automatically once the tests pass.

The 3.7 backport ran into some trouble, probably a simple merge conflict.

Sushma do you want to try your hands at this?  It probably requires some learning about Git branches.  We can also skip this, it's only an improved error message after all, and you can try something more fun instead.

The 3.6 branch is closed except for security fixes, which this isn't.

New changeset 1cc70322c99b80c123f9ff23a415d3da28b4ec74 by Miss Islington (bot) in branch '3.8':
bpo-16970: Adding error message for invalid args (GH-14844)
https://github.com/python/cpython/commit/1cc70322c99b80c123f9ff23a415d3da28b4ec74

Every PR related to this issue (even the ones only referenced during the discussion) was already merged.

Latest message is from more than one year and a half ago. The only thing left to do here would be the backport to 3.7, but according to Guido it could just be skipped (since it's just an improved error message). I don't think Sushma is still interested in this, so I guess this could be closed?

Okay, I trust that this can be closed.