diff -r 9ea84f006892 Lib/argparse.py --- a/Lib/argparse.py Wed May 01 15:15:50 2013 +0200 +++ b/Lib/argparse.py Sun Jul 07 23:33:11 2013 -0700 @@ -136,6 +136,42 @@ setattr(namespace, name, value) return getattr(namespace, name) +def _format_choices(choices, expand=False, summarize=None): + # consolidate the choices formatting in one place + # use formatting as before + # unless choices is not iterable + # in which case the repr() + # could make this an Action method + # another thing to use is the metavar + if hasattr(choices, '__contains__'): + rep = repr(choices) + else: + raise AttributeError('choices must support the in operator') + # or do ' ' in choices, which would raise + # TypeError: argument of type 'instance' is not iterable + try: + choice_strs = [str(choice) for choice in choices] + if summarize: + n = len(choice_strs) + if n>summarize: + split = [6,2] + if summarize<15: + split = [summarize//3,2] + # should tweak this is n is close to 10 + ll = [choice_strs[i] for i in range(0, split[0])] + ll += ['...'] + ll += [choice_strs[i] for i in range(n-split[1],n)] + choice_strs = ll + if expand: + # expanded form used in help + result = ', '.join(choice_strs) + else: + # compact form used in usage + result = '{%s}' % ','.join(choice_strs) + rep = rep.replace(' ', '') + except TypeError: + return rep + return result # =============== # Formatting Help @@ -467,7 +503,10 @@ text = _re.sub(r'(%s) ' % open, r'\1', text) text = _re.sub(r' (%s)' % close, r'\1', text) text = _re.sub(r'%s *%s' % (open, close), r'', text) - text = _re.sub(r'\(([^|]*)\)', r'\1', text) + #text = _re.sub(r'\(([^|]*)\)', r'\1', text) + #text = _re.sub(r'( )\(([^|]*?)\)',r'\1\2', text) + text = _re.sub(r'(?<= )\(([^|]*)\)', r'\1', text) + # remove () from ' (-y)' but not 'range(20)' text = text.strip() # return the text @@ -555,8 +594,9 @@ if action.metavar is not None: result = action.metavar elif action.choices is not None: - choice_strs = [str(choice) for choice in action.choices] - result = '{%s}' % ','.join(choice_strs) + #choice_strs = [str(choice) for choice in action.choices] + #result = '{%s}' % ','.join(choice_strs) + result = _format_choices(action.choices) else: result = default_metavar @@ -582,6 +622,10 @@ elif action.nargs == PARSER: result = '%s ...' % get_metavar(1) else: + # improved nargs testing, issue 9849 + if not isinstance(action.nargs, int): + valid_nargs = [None,OPTIONAL,ZERO_OR_MORE,ONE_OR_MORE,REMAINDER,PARSER] + raise ValueError('nargs %r not integer or %s'%(action.nargs, valid_nargs)) formats = ['%s' for _ in range(action.nargs)] result = ' '.join(formats) % get_metavar(action.nargs) return result @@ -595,7 +639,8 @@ if hasattr(params[name], '__name__'): params[name] = params[name].__name__ if params.get('choices') is not None: - choices_str = ', '.join([str(c) for c in params['choices']]) + #choices_str = ', '.join([str(c) for c in params['choices']]) + choices_str = _format_choices(params['choices'], expand=True) params['choices'] = choices_str return self._get_help_string(action) % params @@ -681,8 +726,6 @@ def _get_default_metavar_for_positional(self, action): return action.type.__name__ - - # ===================== # Options and Arguments # ===================== @@ -1325,12 +1368,9 @@ if not callable(type_func): raise ValueError('%r is not callable' % (type_func,)) - # raise an error if the metavar does not match the type - if hasattr(self, "_get_formatter"): - try: - self._get_formatter()._format_args(action, None) - except TypeError: - raise ValueError("length of metavar tuple does not match nargs") + # issue 9849 + if hasattr(self, "_check_argument"): + self._check_argument(action) return self._add_action(action) @@ -1534,6 +1574,7 @@ self._has_negative_number_optionals = \ container._has_negative_number_optionals self._mutually_exclusive_groups = container._mutually_exclusive_groups + self._check_argument = container._check_argument # issue 9849 def _add_action(self, action): action = super(_ArgumentGroup, self)._add_action(action) @@ -1707,6 +1748,25 @@ for action in self._actions if not action.option_strings] + def _check_argument(self, action): + # check action arguments; issue 9949 + # focus on the arguments that the parent container does not know about + # check nargs and metavar tuple + try: + self._get_formatter()._format_args(action, None) + except (ValueError, AttributeError) as e: + raise ArgumentError(action, _(str(e))) + except TypeError as e: + msg = _(str(e)) + if _re.search(r'argument(.*)format', msg): + msg = _("length of metavar tuple does not match nargs") + """ + possible errors from formatting metavar with nargs + 'not all arguments converted during string formatting', + 'not enough arguments for format string' + """ + raise ArgumentError(action, msg) + # ===================================== # Command line argument parsing methods # ===================================== @@ -2197,6 +2257,8 @@ # all others should be integers else: + if not isinstance(action.nargs, int): # issue 9849 + raise ValueError('nargs %r not integer or valid string'%(action.nargs)) nargs_pattern = '(-*%s-*)' % '-*'.join('A' * nargs) # if this is an optional action, -- is not allowed @@ -2236,7 +2298,11 @@ value = action.default else: value = arg_strings - self._check_value(action, value) + if not isinstance(value, list): + self._check_value(action, value) + else: + for v in value: + self._check_value(action, v) # single argument or optional argument produces a single value elif len(arg_strings) == 1 and action.nargs in [None, OPTIONAL]: @@ -2290,11 +2356,26 @@ 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)') - raise ArgumentError(action, msg % args) + if action.choices is not None: + choices = action.choices + if isinstance(choices, str): + # so 'in' does not find substrings + choices = list(choices) + try: + aproblem = value not in choices + except Exception as e: + msg = _('invalid choice: %r, %s'%(value, e)) + # e.g. None not in 'astring' + raise ArgumentError(action, msg) + if aproblem: + # summarize is # choices exceeds this value + # is there a reasonable way of giving user control of this? + args = {'value': value, + #'choices': ', '.join(map(repr, action.choices)), + 'choices': _format_choices(choices, summarize=15), + } + msg = _('invalid choice: %(value)r (choose from %(choices)s)') + raise ArgumentError(action, msg % args) # ======================= # Help-formatting methods diff -r 9ea84f006892 Lib/test/test_argparse.py --- a/Lib/test/test_argparse.py Wed May 01 15:15:50 2013 +0200 +++ b/Lib/test/test_argparse.py Sun Jul 07 23:33:11 2013 -0700 @@ -627,7 +627,7 @@ """Tests specifying the choices for an Optional""" argument_signatures = [ - Sig('-f', choices='abc'), + Sig('-f', choices=['a', 'b', 'c']), Sig('-g', type=int, choices=range(5))] failures = ['a', '-f d', '-fad', '-ga', '-g 6'] successes = [ @@ -1800,14 +1800,14 @@ parser1_kwargs['aliases'] = ['1alias1', '1alias2'] parser1 = subparsers.add_parser('1', **parser1_kwargs) parser1.add_argument('-w', type=int, help='w help') - parser1.add_argument('x', choices='abc', help='x help') + parser1.add_argument('x', choices=['a', 'b', 'c'], help='x help') # add second sub-parser parser2_kwargs = dict(description='2 description') if subparser_help: parser2_kwargs['help'] = '2 help' parser2 = subparsers.add_parser('2', **parser2_kwargs) - parser2.add_argument('-y', choices='123', help='y help') + parser2.add_argument('-y', choices=['1', '2', '3'], help='y help') parser2.add_argument('z', type=complex, nargs='*', help='z help') # add third sub-parser @@ -3562,7 +3562,7 @@ help='x %(prog)s %(default)s %(type)s %%'), Sig('-y', action='store_const', default=42, const='XXX', help='y %(prog)s %(default)s %(const)s'), - Sig('--foo', choices='abc', + Sig('--foo', choices=['a', 'b', 'c'], help='foo %(prog)s %(default)s %(choices)s'), Sig('--bar', default='baz', choices=[1, 2], metavar='BBB', help='bar %(prog)s %(default)s %(dest)s'), @@ -4720,7 +4720,9 @@ class TestAddArgumentMetavar(TestCase): - EXPECTED_MESSAGE = "length of metavar tuple does not match nargs" + # EXPECTED_MESSAGE = "length of metavar tuple does not match nargs" + EXPECTED_MESSAGE = "argument --foo: length of metavar tuple does not match nargs" + EXPECTED_ERROR = argparse.ArgumentError def do_test_no_exception(self, nargs, metavar): parser = argparse.ArgumentParser() @@ -4728,9 +4730,10 @@ def do_test_exception(self, nargs, metavar): parser = argparse.ArgumentParser() - with self.assertRaises(ValueError) as cm: + # with self.assertRaises(ValueError) as cm: + with self.assertRaisesRegex(self.EXPECTED_ERROR, self.EXPECTED_MESSAGE): parser.add_argument("--foo", nargs=nargs, metavar=metavar) - self.assertEqual(cm.exception.args[0], self.EXPECTED_MESSAGE) + # self.assertEqual(cm.exception.args[0], self.EXPECTED_MESSAGE) # Unit tests for different values of metavar when nargs=None @@ -4885,6 +4888,159 @@ def test_nargs_3_metavar_length3(self): self.do_test_no_exception(nargs=3, metavar=("1", "2", "3")) +# ========================== +# metavar with () +# ========================== + +class TestMetavarWithParen(TestCase): + "MutuallyExclusiveGroup trimming should not remove () from metavars" + def test_default_formatting(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", type=int, choices=range(10)) + cm = parser.format_usage() + self.assertRegex(cm, r"\[-h\] \[--foo {0,1,2,3,4,5,6,7,8,9}\]\n") + + def test_format_with_metavar(self): + # represent choices with the metavar + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", type=int, choices=range(20), metavar='range 20') + cm = parser.format_usage() + self.assertRegex(cm, r'usage: (.*) \[\-h\] \[--foo range 20\]\n') + + def test_format_with_paren_usage(self): + # test a fix that preserves () in the metavar + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", type=int, choices=range(20), metavar='range(0,20)') + cm = parser.format_usage() + self.assertRegex(cm, r'usage: (.*) \[-h\] \[--foo range\(0,20\)\]\n') + + def test_format_with_summarize(self): + # the metavar has no effect on the error message + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", type=int, choices=range(20), metavar='range(0,20)') + self.assertEqual(parser.parse_args(['--foo','1']), NS(foo=1)) + + with self.assertRaises(ArgumentParserError) as cm: + parser.parse_args(['--foo','21']) + msg = str(cm.exception) + self.assertRegex(msg, r'invalid choice') + self.assertNotIn(msg, r'range\(0,20\)') + # range large enough to trigger list summarizing + self.assertRegex(msg, r'choose from {0,1,2,3,4,5,...,18,19}') + + def test_format_with_paren_help(self): + # %(choices)s in help, prefer default over metavar + # this feature is not documented + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", type=int, choices=list(range(10)), + metavar='range(0,10)', help='int from %(choices)s') + cm = parser.format_help() + #print(cm) + self.assertRegex(cm, r'--foo range\(0,10\) int from 0, 1, 2, 3, 4, 5, 6, 7, 8, 9') + +# ========================== +# Non iterable choices +# ========================== + +class TestNonIterableChoices(TestCase): + # partial test of Issue 16418 and 16468 + # choices is a container with __contains__ but not __iter__ + # so words with the 'in' operator, but not 'for in' + class NonIterableContainer: + def __init__(self, container, description): + self.container = container + self.description = description + def __repr__(self): + return self.description + def __contains__(self, item): + return item in self.container + abcChoices = NonIterableContainer(['a','b','c'], 'NIC(abc)') + + def test_use_of_repr(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument('--foo', choices=self.abcChoices) + self.assertEqual(parser.parse_args(['--foo','a']), NS(foo='a')) + cm = parser.format_usage() + self.assertRegex(cm, r'NIC\(abc\)') + + def test_use_of_metavar(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument('--foo', choices=self.abcChoices, metavar='NIC(abc)') + self.assertEqual(parser.parse_args(['--foo','a']), NS(foo='a')) + cm = parser.format_usage() + self.assertRegex(cm, r'\[--foo NIC\(abc\)\]') + + @unittest.skip("fixed error message") + def test_parse_TypeError(self): + # error here should be addressed by issue 16468 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', choices=self.abcChoices, metavar='NIC(abc)') + self.assertEqual(parser.parse_args(['--foo','a']), NS(foo='a')) + with self.assertRaises(ArgumentParserError) as cm: + parser.parse_args(['--foo','d']) + msg = str(cm.exception) + self.assertRegex(msg, "'NonIterableContainer' object is not iterable") + + def test_parse_ArgumentError(self): + # error here should be addressed by issue 16468 + # for code that returns an ArgumentError rather than TypeError + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument('--foo', choices=self.abcChoices, metavar='NIC(abc)') + self.assertEqual(parser.parse_args(['--foo','a']), NS(foo='a')) + with self.assertRaises(ArgumentParserError) as cm: + parser.parse_args(['--foo','d']) + msg = str(cm.exception) + self.assertRegex(msg, r"invalid choice: 'd' \(choose from NIC\(abc\)\)") + + +class TestBareChoices(TestCase): + # test a choices object that does not accept the in operator + # add_argument should raise error + class Bare: + def __repr__(self): + return 'BareObject' + def test_error_raising(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + with self.assertRaises(argparse.ArgumentError) as cm: + parser.add_argument('--foo', type=int,choices=self.Bare()) + msg = str(cm.exception) + self.assertRegex(msg, 'choices must support the in operator') + +# ========================== +# string choices +# ========================== + +class TestStringChoices(TestCase): + "string choices are flaky" + def test_format_expands_to_list(self): + full = r"\[-h\] \[--foo {a,b,c}\]\n" + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", choices='abc') + cm = parser.format_usage() + self.assertRegex(cm, full) + + def test_parse_admits_substring_not(self): + # 'bc' in 'abc' is True, but 'bc' in list('abc') is not + # _check_value now converts 'abc' to list + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", choices='abc') + self.assertEqual(parser.parse_args(['--foo','a']), NS(foo='a')) + #self.assertEqual(parser.parse_args(['--foo','bc']), NS(foo='bc')) + with self.assertRaises(ArgumentParserError) as cm: + parser.parse_args(['--foo','bc']) + msg = str(cm.exception) + self.assertRegex(msg, "invalid choice: 'bc' \(choose from {a,b,c}\)") + + def test_parse_list_does_not_admit_substring(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument("--foo", choices=list('abc')) + self.assertEqual(parser.parse_args(['--foo','a']), NS(foo='a')) + # self.assertEqual(parser.parse_args(['--foo','bc']), NS(foo='bc')) + with self.assertRaises(ArgumentParserError) as cm: + parser.parse_args(['--foo','bc']) + msg = str(cm.exception) + self.assertRegex(msg, "invalid choice: 'bc' \(choose from {a,b,c}\)") + # ============================ # from argparse import * tests # ============================