homepage

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.

classification
Title: argparse: mismatch between choices parsing and usage/error message
Type: behavior Stage: test needed
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: chris.jerdonek, eric.smith, iritkatriel, jeffknupp, paul.j3
Priority: normal Keywords: patch

Created on 2013年01月16日 02:30 by chris.jerdonek, last changed 2022年04月11日 14:57 by admin.

Files
File name Uploaded Description Edit
argparse.patch jeffknupp, 2013年01月18日 15:51 review
Messages (13)
msg180068 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013年01月16日 02:30
When passing a string for the choices argument, argparse's usage and error messages differ from its behavior:
>>> p = argparse.ArgumentParser()
>>> p.add_argument('a', choices='abc')
>>> p.parse_args(['d'])
usage: [-h] {a,b,c}
: error: argument a: invalid choice: 'd' (choose from 'a', 'b', 'c')
>>> p.parse_args(['bc'])
Namespace(a='bc')
This is because argparse uses the "in" operator instead of sequence iteration to check whether an argument value is valid. Any resolution should also consider the behavior for string subclasses as well as perhaps bytes-like objects.
msg180069 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013年01月16日 02:40
I forgot to mention that argparse uses such cases as examples in its documentation (one of which was replaced in bddbaaf332d7).
msg180136 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2013年01月17日 14:55
Isn't this really just an inappropriate use of a string instead of a list? If indeed this is in the documentation, it should be changed.
I still don't like:
>>> p.add_argument('a', choices=list('abc'))
but at least it would work.
This call to list() could be done internally, but I think passing in a string is a bad practice and argparse should not contain internal workarounds to cater to this usage.
If you're proposing that argparse should use sequence iteration instead of the "in" operator, I disagree with that solution.
msg180153 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013年01月17日 20:48
This wasn't just in the documentation. It was the *first* example of how to use the choices argument out of the two examples in that section (from the time Python first adopted argparse and before):
----
16.4.3.7. choices
Some command-line arguments should be selected from a restricted set of values. These can be handled by passing a container object as the choices keyword argument to add_argument(). When the command line is parsed, argument values will be checked, and an error message will be displayed if the argument was not one of the acceptable values:
>>> parser = argparse.ArgumentParser(prog='PROG')
>>> parser.add_argument('foo', choices='abc')
>>> parser.parse_args('c'.split())
Namespace(foo='c')
>>> parser.parse_args('X'.split())
usage: PROG [-h] {a,b,c}
PROG: error: argument foo: invalid choice: 'X' (choose from 'a', 'b', 'c')
(from http://hg.python.org/cpython/file/c0ddae67f4df/Doc/library/argparse.rst#l1021 )
----
So I think it's a bit late to say it's an inappropriate usage or bad practice.
To preserve backwards compatibility, I think we should use sequence iteration for strings, or equivalently apply "in" to iter(choices), set(choices), etc. when choices is a string. I don't think, however, that we should alter the choices attribute because that could break things for people:
>>> p = argparse.ArgumentParser()
>>> a = p.add_argument("letter", choices='abcd')
>>> a.choices
'abcd'
msg180187 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013年01月18日 11:07
There are also test cases with a string being passed for choices.
msg180197 - (view) Author: Jeff Knupp (jeffknupp) * Date: 2013年01月18日 15:51
Attached a patch. Rather than altering choices or making a special check for string instances, I just changed the if statement to
if action.choices is not None and value not in list(action.choices):
from 
if action.choices is not None and value not in action.choices:
It has the added benefit of handling all sequence types correctly (rather than just strings). I tried to think of a case where this wouldn't work as expected, but wasn't able to.
msg180208 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013年01月18日 18:32
See issue 16468 for why that won't work in general.
msg180216 - (view) Author: Jeff Knupp (jeffknupp) * Date: 2013年01月18日 19:38
The only time this would be an issue is for infinite sequences via range or a generator, which doesn't work anyway.
>>> p = argparse.ArgumentParser()
>>> a = p.add_argument('a', choices=itertools.count(0), type=int)
>>> p.parse_args(['10000'])
... hangs
Are there any other cases where coercing to a list wouldn't work?
msg180218 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2013年01月18日 20:23
When choices isn't iterable but supports the "in" operator, e.g.
class MyChoices(object):
 def __contains__(self, item):
 return True
msg192254 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013年07月03日 20:06
test_argparse.py has some "choices='abc'" cases. 
In those should "parser.parse_args(['--foo','bc'])" be considered a success or failure? 
The underlying issue here is that while string iteration behaves like list iteration, string __contains__ looks for substrings, not just one character that matches. (String __contains__ also returns a TypeError if its argument is not a string.)
But other than removing poor examples in documentation and tests, I'm not sure this issue requires a change.
msg192295 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013年07月04日 15:09
Changing _check_value from:
 def _check_value(self, action, value):
 # converted value must be one of the choices (if specified)
 if action.choices is not None and value not in action.choices:
 ...
to
 def _check_value(self, action, value):
 # converted value must be one of the choices (if specified)
 if action.choices is not None:
 choices = action.choices
 if isinstance(choices, str):
 choices = list(choices)
 if value not in action.choices:
 ...
would correct the string search without affecting other types of choices.
msg192715 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013年07月09日 03:06
I just posted a patch to http://bugs.python.org/issue16468 that deals with this 'bc' in 'abc' issue.
msg408213 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021年12月10日 15:38
We could make the error message less wrong:
>>> p.parse_args(['d'])
usage: [-h] {a,b,c}
: error: argument a: invalid choice: 'd' (choose a value in 'abc')
% git diff
diff --git a/Lib/argparse.py b/Lib/argparse.py
index b44fa4f0f6..f03cc1f110 100644
--- a/Lib/argparse.py
+++ b/Lib/argparse.py
@@ -2499,8 +2499,8 @@ def _check_value(self, action, value):
 # converted value must be one of the choices (if specified)
 if action.choices is not None and value not in action.choices:
 args = {'value': value,
- 'choices': ', '.join(map(repr, action.choices))}
- msg = _('invalid choice: %(value)r (choose from %(choices)s)')
+ 'choices': repr(action.choices)}
+ msg = _('invalid choice: %(value)r (choose a value in %(choices)s)')
 raise ArgumentError(action, msg % args)
 
 # =======================
History
Date User Action Args
2022年04月11日 14:57:40adminsetgithub: 61181
2021年12月10日 15:38:13iritkatrielsetnosy: + iritkatriel

messages: + msg408213
versions: + Python 3.9, Python 3.10, Python 3.11, - Python 2.7, Python 3.2, Python 3.3, Python 3.4
2013年07月09日 03:06:38paul.j3setmessages: + msg192715
2013年07月04日 15:09:04paul.j3setmessages: + msg192295
2013年07月03日 20:06:52paul.j3setnosy: + paul.j3
messages: + msg192254
2013年01月18日 20:23:16chris.jerdoneksetmessages: + msg180218
2013年01月18日 19:38:31jeffknuppsetmessages: + msg180216
2013年01月18日 18:32:43chris.jerdoneksetmessages: + msg180208
2013年01月18日 15:51:21jeffknuppsetfiles: + argparse.patch

nosy: + jeffknupp
messages: + msg180197

keywords: + patch
2013年01月18日 11:07:46chris.jerdoneksetmessages: + msg180187
2013年01月17日 20:48:46chris.jerdoneksetmessages: + msg180153
2013年01月17日 14:55:06eric.smithsetnosy: + eric.smith
messages: + msg180136
2013年01月16日 02:40:38chris.jerdoneksetmessages: + msg180069
2013年01月16日 02:30:07chris.jerdonekcreate

AltStyle によって変換されたページ (->オリジナル) /