msg180068 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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)
# =======================
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:40 | admin | set | github: 61181 |
2021-12-10 15:38:13 | iritkatriel | set | nosy:
+ 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:38 | paul.j3 | set | messages:
+ msg192715 |
2013-07-04 15:09:04 | paul.j3 | set | messages:
+ msg192295 |
2013-07-03 20:06:52 | paul.j3 | set | nosy:
+ paul.j3 messages:
+ msg192254
|
2013-01-18 20:23:16 | chris.jerdonek | set | messages:
+ msg180218 |
2013-01-18 19:38:31 | jeffknupp | set | messages:
+ msg180216 |
2013-01-18 18:32:43 | chris.jerdonek | set | messages:
+ msg180208 |
2013-01-18 15:51:21 | jeffknupp | set | files:
+ argparse.patch
nosy:
+ jeffknupp messages:
+ msg180197
keywords:
+ patch |
2013-01-18 11:07:46 | chris.jerdonek | set | messages:
+ msg180187 |
2013-01-17 20:48:46 | chris.jerdonek | set | messages:
+ msg180153 |
2013-01-17 14:55:06 | eric.smith | set | nosy:
+ eric.smith messages:
+ msg180136
|
2013-01-16 02:40:38 | chris.jerdonek | set | messages:
+ msg180069 |
2013-01-16 02:30:07 | chris.jerdonek | create | |