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.

Author terry.reedy
Recipients Juraj.Variny, bethard, chris.jerdonek, docs@python, ezio.melotti, r.david.murray, terry.reedy
Date 2012-11-13.19:32:43
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1352835165.22.0.00473803487316.issue16418@psf.upfronthosting.co.za>
In-reply-to
Content
I do not agree with the patch. A summary of my view: Range objects support the 'in' operator and they are an intended option for choices, and, as I said before, are exactly the right option for arithmetic sequences with more than a few items. The problem is that they are now, in effect, special-cased relative to other builtins by having their compact representation replaced by an expanded tuple display. Moreover, the iteration required to do this introduces a discrepancy relative to the doc. This bug might be better fixed by a code change.

(The OP did not pass a (x)range object but a list. That was unnecessary and easily fixed in itself. But such a fix leaves the issue above. Condensing long sequences is a somewhat separate issue.)

As to intent:

"The choices keyword argument may be more convenient for type checkers that simply check against a range of values:
>>>

>>> parser = argparse.ArgumentParser(prog='PROG')
>>> parser.add_argument('foo', type=int, choices=range(5, 10))
>>> parser.parse_args('7'.split())
Namespace(foo=7)
>>> parser.parse_args('11'.split())
usage: PROG [-h] {5,6,7,8,9}
PROG: error: argument foo: invalid choice: 11 (choose from 5, 6, 7, 8, 9)"

Note the expansion instead of the normal representation. It is not a big deal here, but obviously can be.

">>> parser.add_argument(
...     'integers', metavar='int', type=int, choices=range(10),
...  nargs='+', help='an integer in the range 0..9')"

As to tuple display expansion: the link points to

2284 def _check_value(self, action, value):
2285   # converted value must be one of the choices (if specified)
2286   if action.choices is not None and value not in action.choices:
2287     args = {'value': value,
2288     'choices': ', '.join(map(repr, action.choices))}
2289     msg = _('invalid choice: %(value)r (choose from %(choices)s)')
2290     raise ArgumentError(action, msg % args)

In 2288 "', '.join(map(repr, action.choices))" produces a tuple display without parentheses. It essentially reproduced str/repr for tuples, lists, frozensets, sets, dicts, dict views, etc., leaving off the irrelevant fence characters. In doing so, by iteration, it introduces a bug --see below.

For range objects, the tuple representation is a drastic change from the normal representation. In that sense, it special cases range among built-ins, and in a bad way when the range represents many values. (Help messages apparently do the same.) I consider this to be something of a bug. So the code as it is would have to special case range objects to avoid special-casing them in the sense above.

The same would apply to any custom objects that have a succinct description for a large, possible infinite set. Here are two examples: "a word containing no 'e's" and "a 'word' containing only the letters a, b, c, d, e". Objects representing such infinite sets of strings could easily have a __contains__ method but not an __iter__ method.

The code above requires the choices object to be iterable as well as supporting 'in'. This contradicts the doc statement "Any object that supports the in operator can be passed as the choices value,". That discrepancy is a bug. It should be fixed by either adding the restriction to the doc or removing it from the code. I recommend the latter.

The code could simply use the str or repr of the choice object without trying to be fancy with a custom, fence-stripped, representation that does not work correctly or at all for all possible choice objects. In other words

if action.choices is not None and value not in action.choices:
  msg = "invalid choice: %r (choose from %r)" % (value, action.choices)
  raise ArgumentError(action, msg)

If the custom representation for non-range builtins is desired, then they are the ones that should be special-cased to not use their default representation.
History
Date User Action Args
2012-11-13 19:32:45terry.reedysetrecipients: + terry.reedy, bethard, ezio.melotti, r.david.murray, chris.jerdonek, docs@python, Juraj.Variny
2012-11-13 19:32:45terry.reedysetmessageid: <1352835165.22.0.00473803487316.issue16418@psf.upfronthosting.co.za>
2012-11-13 19:32:45terry.reedylinkissue16418 messages
2012-11-13 19:32:43terry.reedycreate