Title: Calling argparse's add_argument with the wrong number of metavars causes delayed error message
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: bethard Nosy List: bethard, catherine, denversc
Priority: normal Keywords: patch

Created on 2010-07-23 13:34 by bethard, last changed 2011-03-27 01:04 by denversc. This issue is now closed.

File name Uploaded Description Edit
wrong_metavars_test.patch catherine, 2010-08-02 17:44 review
wrong_metavars_test_corrected.patch catherine, 2010-08-04 18:14 review
issue9348_patch_v01.txt denversc, 2011-03-19 19:08 patch in cpython tree at changeset bb645cc39e60 review
Messages (8)
msg111317 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2010-07-23 13:34
What steps will reproduce the problem?

parser = argparse.ArgumentParser()	
parser.add_argument('--foo', nargs=2, metavar=('X','Y','Z'))

The error dosn't show up until help is formatted.

Giving any incorrect length of metavar will produce the problem, which
includes a tuple whos length doesn't match a numerical value, more than two metavars to '*' or '+', and more than one metavar to '?'. Furthermore, a tuple of length one causes the error when nargs is greater than 1, '*', or '+'.

What is the expected output? What do you see instead?

When the help is displayed, you get:

TypeError: not all arguments converted during string formatting

Or a similar error message for other cases.

It would be expected that the error message would be more specific. The
error should definitely be raised when add_argument is called, rather than later.

There should be a test that does something like:

    for meta in ('X', ('X',), ('X','Y'), ('X','Y','Z')):
        for n in (1, 2, 3, '?', '+', '*'):
            parser = argparse.ArgumentParser()
            parser.add_argument('--foo', nargs=n, metavar=meta)

and makes sure that the error shows up in add_argument, not format_help.
msg112503 - (view) Author: Catherine Devlin (catherine) Date: 2010-08-02 17:44
I'm attaching a unit test patch that does something vaguely like Steven suggests.  It definitely needs review.

Also, I'm assuming that it's OK to specify *fewer* metavars than nargs, just not more... but I'm not sure about that.
msg112774 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2010-08-04 08:56
You can specify either 1 or N. So for n=3, you can specify metavar="X" or metavar=("X", "Y", "Z") but not metavar=("X", "Y"). The special nargs value "?" always takes only one, while "*" and "+" always take two. (This makes sense if you think about how they're formatted, e.g. "X [X ...]".)
msg112849 - (view) Author: Catherine Devlin (catherine) Date: 2010-08-04 18:14
Thanks for the correction, Steven.  This unit test matches your description.  Use it instead of my earlier submission.
msg131439 - (view) Author: Denver Coneybeare (denversc) * Date: 2011-03-19 19:08
For kicks, I just took a look at this old, forgotten issue.  I agree with the submitter that add_argument() should fail if nargs and metavar do not match, instead of having format_help() raise the exception later on.

I've attached a patch (issue9348_patch_v01.txt) with a proposed fix and associated test cases.  The drawback of my approach is that if a custom HelpFormatter is used which has different semantics for metavar it could cause add_argument() to incorrectly reject the metavar value.  I'm not sure how common that is though.
msg132248 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-26 16:59
New changeset c89db9b36ea6 by Steven Bethard in branch '3.2':
Issue #9348: Raise an early error if argparse nargs and metavar don't match.

New changeset b93a50bb74f2 by Steven Bethard in branch 'default':
Issue #9348: Raise an early error if argparse nargs and metavar don't match. (Merge from 3.2.)

New changeset 4bb651eb7539 by Steven Bethard in branch '2.7':
Issue #9348: Raise an early error if argparse nargs and metavar don't match. (Merge from 3.2.)
msg132250 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2011-03-26 17:02
Thanks for the patch. I used something similar to what you proposed, but instead of creating a local formatter, I just call self._get_formatter() if it exists.
msg132282 - (view) Author: Denver Coneybeare (denversc) * Date: 2011-03-27 01:04
Awesome, thanks for committing the patch.  Glad I could help.
Date User Action Args
2011-03-27 01:04:19denverscsetmessages: + msg132282
2011-03-26 17:02:24bethardsetstatus: open -> closed

assignee: bethard
versions: + Python 3.3
nosy: - python-dev

messages: + msg132250
resolution: fixed
stage: needs patch -> resolved
2011-03-26 16:59:14python-devsetnosy: + python-dev
messages: + msg132248
2011-03-19 19:08:58denverscsetfiles: + issue9348_patch_v01.txt
nosy: + denversc
messages: + msg131439

2010-08-04 18:14:21catherinesetfiles: + wrong_metavars_test_corrected.patch

messages: + msg112849
2010-08-04 08:56:32bethardsetmessages: + msg112774
2010-08-02 17:44:42catherinesetfiles: + wrong_metavars_test.patch

nosy: + catherine
messages: + msg112503

keywords: + patch
2010-07-23 13:34:14bethardcreate