python / cpython

The Python programming language
https://www.python.org
Other
61.99k stars 29.81k forks source link

argparse allows nargs>1 for positional arguments but doesn't allow metavar to be a tuple #58282

Open ccda776a-4736-4dbd-bf35-88e58f6952ba opened 12 years ago

ccda776a-4736-4dbd-bf35-88e58f6952ba commented 12 years ago
BPO 14074
Nosy @rhettinger, @cykerway, @shihai1991, @iritkatriel
PRs
  • python/cpython#10847
  • Files
  • issue14074.patch
  • issue14074_1.patch
  • 14074.patch: lightweight patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.9', '3.10', '3.11'] title = "argparse allows nargs>1 for positional arguments but doesn't allow metavar to be a tuple" updated_at = user = 'https://bugs.python.org/tshepang' ``` bugs.python.org fields: ```python activity = actor = 'iritkatriel' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'tshepang' dependencies = [] files = ['31042', '35049', '47965'] hgrepos = [] issue_num = 14074 keywords = ['patch'] message_count = 13.0 messages = ['153884', '154236', '193737', '217184', '217240', '286227', '330854', '360841', '361154', '361184', '361215', '361336', '407109'] nosy_count = 8.0 nosy_names = ['rhettinger', 'bethard', 'tshepang', 'paul.j3', 'vvas', 'cykerway', 'shihai1991', 'iritkatriel'] pr_nums = ['10847'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue14074' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    ccda776a-4736-4dbd-bf35-88e58f6952ba commented 12 years ago

    Here's four commands to demonstrate my problem:

    My sample code: $ cat prog.py import argparse parse = argparse.ArgumentParser() parse.add_argument("foo", nargs=2, metavar=("foo", "bar")) args = parse.parse_args() print(args)

    This output could be friendlier:
    $ python3 prog.py --help
    Traceback (most recent call last):
      File "prog.py", line 4, in <module>
        args = parse.parse_args()
      File "/home/wena/cpython/Lib/argparse.py", line 1722, in parse_args
        args, argv = self.parse_known_args(args, namespace)
      File "/home/wena/cpython/Lib/argparse.py", line 1754, in parse_known_args
        namespace, args = self._parse_known_args(args, namespace)
      File "/home/wena/cpython/Lib/argparse.py", line 1960, in _parse_known_args
        start_index = consume_optional(start_index)
      File "/home/wena/cpython/Lib/argparse.py", line 1900, in consume_optional
        take_action(action, args, option_string)
      File "/home/wena/cpython/Lib/argparse.py", line 1828, in take_action
        action(self, namespace, argument_values, option_string)
      File "/home/wena/cpython/Lib/argparse.py", line 1015, in __call__
        parser.print_help()
      File "/home/wena/cpython/Lib/argparse.py", line 2347, in print_help
        self._print_message(self.format_help(), file)
      File "/home/wena/cpython/Lib/argparse.py", line 2314, in format_help
        formatter.add_arguments(action_group._group_actions)
      File "/home/wena/cpython/Lib/argparse.py", line 270, in add_arguments
        self.add_argument(action)
      File "/home/wena/cpython/Lib/argparse.py", line 255, in add_argument
        invocations = [get_invocation(action)]
      File "/home/wena/cpython/Lib/argparse.py", line 533, in _format_action_invocation
        metavar, = self._metavar_formatter(action, default)(1)
    ValueError: too many values to unpack (expected 1)
    
    On Python 3.2 (from Debian), I get "error: too few arguments". But with latest mercurial I get:
    $ python3 prog.py
    Traceback (most recent call last):
      File "prog.py", line 4, in <module>
        args = parse.parse_args()
      File "/home/wena/cpython/Lib/argparse.py", line 1722, in parse_args
        args, argv = self.parse_known_args(args, namespace)
      File "/home/wena/cpython/Lib/argparse.py", line 1754, in parse_known_args
        namespace, args = self._parse_known_args(args, namespace)
      File "/home/wena/cpython/Lib/argparse.py", line 1973, in _parse_known_args
        ', '.join(required_actions))
    TypeError: sequence item 0: expected str instance, tuple found

    But this looks okay: $ python3 prog.py a b Namespace(foo=['a', 'b'])

    8955c213-fd54-471c-9758-9cc5f49074db commented 12 years ago

    Looks like the problem is that "_format_action_invocation" is not being as careful with the different possibilities for metavar as "_format_args" is. Patches welcome!

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 11 years ago

    This patch fixes the problem by joining the metavar terms with '|'. So the help for the test case (adapted from an existing tuple test) looks like:

    usage: PROG [-h] W1 [W2 ...] [X1 [X2 ...]] Y1 Y2 Y3 [Z1]
    positional arguments:
      W1|W2       w
      X1|X2       x
      Y1|Y2|Y3    y
      Z1          z

    Alternatives include:

    The last alternative would use:

        #metavar = '|'.join(metavar)
        if len(metavar)>1:
            metavar = default
        else:
            metavar = metavar[0]
    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    oops - to fix the error message that OP complained about, I need to patch '_get_action_name' as well:

        def _get_action_name(argument):
        ...
            elif argument.metavar not in (None, SUPPRESS):
                metavar = argument.metavar
                if isinstance(metavar, tuple):
                    metavar = '|'.join(metavar)
                return metavar
    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    This patch fixes both help and error formatting.

    A module level '_format_metavars' does the formatting for both.

    I have tried several alternatives, including using the 'usage' style.

    There is similarity between this fix and that for bpo-16468 (custom choices), though I don't think they conflict. In both cases, code needed to format the usage or help is also needed to help format error messages.

    bpo-9849 (better nargs warning) is another case where error checking in the parser depends on the formatter. In the long run we may want to refactor these issues.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 7 years ago

    An alternative fix is to disallow tuple metavars for positionals.

    A tuple metavar may look nice in the usage. But a well selected dest seems better in the help line, and error messages. Using dest in all 3 roles, as we do now, is the simplest compromise.

    73bed6b7-9e64-4c8b-81bb-f55258f94c70 commented 5 years ago

    Can confirm this bug still exists on master branch, python3.7, python3.6, and very likely other versions since it's reported.

    It seems only _format_action_invocation and _get_action_name need to be fixed. So we can do it more lightweight (<10 lines).

    rhettinger commented 4 years ago

    -0 on going forward with this. AFAICT, no one has requested tuple support (the OP just wanted better error handling). And more fined grained control can already be had just by specifying separate positional arguments.

    Paul suggested that it may be a better plan to just disallow tuple metavars and to give a better error message as the OP suggested. That makes sense to me.

    shihai1991 commented 4 years ago

    I am not sure disallowing tuple metavars would break backup compatibility or not\~

    rhettinger commented 4 years ago

    I am not sure disallowing tuple metavars would break backup compatibility or not\~

    Given that tuple metavars currently doesn't work, there is no backward compatibility issue. What is being proposed is to give more friendly error messages.

    shihai1991 commented 4 years ago

    Given that tuple metavars currently doesn't work, there is no backward compatibility issue. What is being proposed is to give more friendly error messages.

    Got it, thanks.

    Paul suggested that it may be a better plan to just disallow tuple metavars and to give a better error message as the OP suggested. That makes sense to me.

    +1

    73bed6b7-9e64-4c8b-81bb-f55258f94c70 commented 4 years ago

    Tuple support is documented:

    https://github.com/python/cpython/commit/98047eb806227f11212f6a42c242030a51964e30#diff-9c4a053d29149ba40370fbdddd3e34faR1059

    https://docs.python.org/3/library/argparse.html#metavar

    Providing a tuple to metavar specifies a different display for each of the arguments:

    From his word here I feel like our professor meant to support tuple metavar but the implementation was not careful. He called for a patch eight years ago. Let's summon him and see if he has changed his mind?

    iritkatriel commented 2 years ago

    Reproduced on 3.11, with a different error:

    % ./python.exe prog.py
    Traceback (most recent call last):
      File "/Users/iritkatriel/src/cpython-1/prog.py", line 5, in <module>
        args = parse.parse_args()
               ^^^^^^^^^^^^^^^^^^
      File "/Users/iritkatriel/src/cpython-1/Lib/argparse.py", line 1822, in parse_args
        args, argv = self.parse_known_args(args, namespace)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/iritkatriel/src/cpython-1/Lib/argparse.py", line 1855, in parse_known_args
        namespace, args = self._parse_known_args(args, namespace)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/Users/iritkatriel/src/cpython-1/Lib/argparse.py", line 2093, in _parse_known_args
        ', '.join(required_actions))
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    TypeError: sequence item 0: expected str instance, tuple found
    kkarbowiak commented 1 year ago

    Hi All.

    I have stumbled upon this issue almost three years ago and wrote a question on StackOverflow.

    My sample program was

    import argparse
    
    parser = argparse.ArgumentParser()
    parser.add_argument('size', type=int, nargs=2, help='size', metavar=('w', 'h'))
    
    args = parser.parse_args()
    print(args)

    When the program is called as prog.py --help it produces an error that varies between Python3 versions (I tried 3.5, 3.6, and 3.8) which can be ValueError: too many values to unpack (expected 1) or TypeError: sequence item 0: expected str instance, tuple found.

    For optional arguments

    import argparse
    
    parser = argparse.ArgumentParser()
    parser.add_argument('--size', type=int, nargs=2, help='size', metavar=('w', 'h'))
    
    args = parser.parse_args()
    print(args)

    all is fine

    usage: prog.py [-h] [--size w h]
    
    optional arguments:
      -h, --help  show this help message and exit
      --size w h  size

    The official documentation mentions using tuples in metavar, but only provides an example for optional arguments.

    I would argue that positional arguments should behave the same and would rather see this fixed than banned.