This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2010年11月15日 10:38 by bethard, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| bug10424.py | maker, 2010年11月20日 00:07 | |||
| issue10424.patch | maker, 2010年11月21日 11:24 | review | ||
| issue10424_2.patch | maker, 2011年05月26日 17:43 | review | ||
| issue10424.patch | maker, 2011年05月27日 17:50 | review | ||
| Messages (14) | |||
|---|---|---|---|
| msg121220 - (view) | Author: Steven Bethard (bethard) * (Python committer) | Date: 2010年11月15日 10:38 | |
From a private email in respect to the following class of error messages:
>>> parser = argparse.ArgumentParser(prog='PROG')
>>> parser.add_argument('--foo')
>>> parser.add_argument('--bar')
>>> parser.add_argument('ham')
>>> parser.add_argument('spam', nargs='+')
>>> parser.parse_args(['HAM'])
usage: PROG [-h] [--foo FOO] [--bar BAR] ham spam [spam ...]
PROG: error: too few arguments
----------------------------------------------------------------------
One suggestion would be that when it displays the error "too few arguments", it would nice if it said something about the argument(s) that are missing.
I modified argparse's error message when there are too few arguments. I didn't examine the code a lot, so there might be cases where this doesn't work, but here's what I did:
if positionals:
self.error(_('too few arguments: %s is required' % positionals[0].dest))
----------------------------------------------------------------------
This would be a nice feature - I haven't checked if the suggested approach works in general though.
|
|||
| msg121575 - (view) | Author: Michele Orrù (maker) * | Date: 2010年11月19日 22:03 | |
This issue seems already fixed. File: Lib/argparse.py 922 # if we didn't use all the Positional objects, there were too few 1923 # arg strings supplied. 1924 if positionals: 1925 self.error(_('too few arguments')) 1926 1927 # make sure all required actions were present 1928 for action in self._actions: 1929 if action.required: 1930 if action not in seen_actions: 1931 name = _get_action_name(action) 1932 self.error(_('argument %s is required') % name) |
|||
| msg121577 - (view) | Author: Steven Bethard (bethard) * (Python committer) | Date: 2010年11月19日 22:40 | |
No, it's exactly line 1925 that's the problem. The OP would like that to tell him which arguments were missing instead of saying just 'too few arguments'. The block below that is for checking required optionals/positionals. It won't execute if the self.error above it happens. |
|||
| msg121580 - (view) | Author: Michele Orrù (maker) * | Date: 2010年11月20日 00:06 | |
The attached patch solves this issue. I haven't added any unittest because test_argparse.py is quite huge - over 4300 lines-, and I was undecided between «ArgumentError tests» (4251) and «ArgumentTypeError tests» (4262). Any hint? However, file bug10424.py reproduces this bug. |
|||
| msg121593 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2010年11月20日 03:57 | |
There are currently no tests in argparse that test the content of error messages, which is fairly standard for stdlib tests since the error messages aren't considered part of the API (only the nature of the exception is). So there's really no existing test class in test_argparse that would be an appropriate place to put a unit test for this. I would instead create a new class. Perhaps TestErrorContent? For this test it would probably be best to use assertRaisesRegexp and just check to make sure that the expected list of argument names appears in the message (that is, don't test the contents of the remainder of the message). You should test several combinations. (The argparse tests do some fancy footwork around errors, though, so you may not be able to use assertRaisesRegexp directly). Your approach to the patch, by the way, is an interesting one, and seems to me to make sense. But I'm not 100% sure I understand the logic of the code you are modifying, so I'll leave it to Steven to comment on that aspect of it :) Your reference to TestArgumentError reveals a minor bug in the tests: there are actually two test classes named TestArgumentError. Presumably the second one was indeed supposed to be TestArgumentTypeError. I've fixed that in r86542. |
|||
| msg121845 - (view) | Author: Steven Bethard (bethard) * (Python committer) | Date: 2010年11月21日 03:06 | |
Yeah a new test class is fine. And I checked the patch and it looks okay to me. My first thought was also "wait does that really work?" but I see that positionals are all marked as required when appropriate (look for the comment starting with "mark positional arguments as required"). I don't have time to test the patch right now, but if someone else does, I'm fine with this after the test for the new behavior is added. |
|||
| msg121887 - (view) | Author: Michele Orrù (maker) * | Date: 2010年11月21日 09:17 | |
Unittest added. |
|||
| msg121904 - (view) | Author: Michele Orrù (maker) * | Date: 2010年11月21日 11:24 | |
Ezio reviewed my patch; here there's the new version with some improvements. |
|||
| msg136981 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2011年05月26日 16:49 | |
I would remove the docstring from the new test class...if more tests of message content are added that docstring won't be accurate. It really isn't needed. (Also, shouldn't the test method be named test_missingarguments?) I would also like to see a test where there are optional non-option arguments (nargs='?'), where you test that the optional one does *not* appear in the message. That'll give you a second test method, making that test class a *little* less trivial :) |
|||
| msg136986 - (view) | Author: Michele Orrù (maker) * | Date: 2011年05月26日 17:43 | |
Done. |
|||
| msg137080 - (view) | Author: Éric Araujo (eric.araujo) * (Python committer) | Date: 2011年05月27日 16:31 | |
FYI, you can upload versions of the same patch with the same name and remove old versions. The code review tool will remember versions, and it’s easier for human if there’s only one patch. |
|||
| msg137086 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2011年05月27日 17:02 | |
What I had in mind for the second test was something that did this (which I think is legal from reading the docs):
parser.add_argument('foo')
parser.add_argument('bar', nargs='?', default='eggs')
with assertRaisesRegex(ArgumentParseError) as cm:
parser.parse_args([])
self.assertNotIn('bar', str(cm.exception))
|
|||
| msg138010 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2011年06月09日 16:34 | |
New changeset cab204a79e09 by R David Murray in branch 'default': #10424: argument names are now included in the missing argument message http://hg.python.org/cpython/rev/cab204a79e09 |
|||
| msg138014 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2011年06月09日 16:38 | |
With input from Michele on IRC I updated the tests to be more generic (not depend on the order of the reported argument names) and moved the test I talked about in my last message into a third unit test. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:08 | admin | set | github: 54633 |
| 2011年06月09日 16:38:26 | r.david.murray | set | status: open -> closed type: enhancement messages: + msg138014 resolution: accepted stage: needs patch -> resolved |
| 2011年06月09日 16:34:42 | python-dev | set | nosy:
+ python-dev messages: + msg138010 |
| 2011年05月27日 17:50:30 | maker | set | files: + issue10424.patch |
| 2011年05月27日 17:02:27 | r.david.murray | set | messages: + msg137086 |
| 2011年05月27日 16:31:11 | eric.araujo | set | messages:
+ msg137080 versions: + Python 3.3, - Python 3.2 |
| 2011年05月26日 17:43:37 | maker | set | files:
+ issue10424_2.patch messages: + msg136986 |
| 2011年05月26日 16:49:43 | r.david.murray | set | messages: + msg136981 |
| 2010年11月21日 11:24:32 | maker | set | files: - issue10424.patch |
| 2010年11月21日 11:24:15 | maker | set | files:
+ issue10424.patch messages: + msg121904 |
| 2010年11月21日 09:17:56 | maker | set | files: - issue10424.patch |
| 2010年11月21日 09:17:43 | maker | set | files:
+ issue10424.patch messages: + msg121887 |
| 2010年11月21日 03:06:45 | bethard | set | messages: + msg121845 |
| 2010年11月20日 13:14:24 | eric.araujo | set | nosy:
+ eric.araujo |
| 2010年11月20日 03:57:58 | r.david.murray | set | nosy:
+ r.david.murray messages: + msg121593 |
| 2010年11月20日 00:07:19 | maker | set | files: + bug10424.py |
| 2010年11月20日 00:06:59 | maker | set | files:
+ issue10424.patch keywords: + patch messages: + msg121580 |
| 2010年11月19日 22:40:44 | bethard | set | messages: + msg121577 |
| 2010年11月19日 22:03:40 | maker | set | nosy:
+ ezio.melotti, maker messages: + msg121575 |
| 2010年11月15日 10:38:42 | bethard | create | |