Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

argparse assertion failure with brackets in metavars #56083

Closed
htnieman mannequin opened this issue Apr 19, 2011 · 23 comments
Closed

argparse assertion failure with brackets in metavars #56083

htnieman mannequin opened this issue Apr 19, 2011 · 23 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@htnieman
Copy link
Mannequin

htnieman mannequin commented Apr 19, 2011

BPO 11874
Nosy @ncoghlan, @merwok, @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll, @wimglenn, @vajrasky, @wm75, @spaceone, @miss-islington
PRs
  • bpo-11874 argparse assertion failure #1826
  • [3.7] bpo-11874: fix assertion failure in argparse metavar handling (GH-1826) #7527
  • [3.6] bpo-11874: fix assertion failure in argparse metavar handling (GH-1826) #7528
  • [3.7] bpo-11874: fix assertion failure in argparse metavar handling (GH-1826) #7530
  • [2.7] bpo-11874: fix assertion failure in argparse metavar handling (GH-1826) #7553
  • Files
  • issue_11874.diff
  • unit_test_for_inner_bracket_metavar.txt: Unit test
  • unit_test_argparse.txt: Revamped unit test
  • fix_and_unit_test_for_argparse_inner_bracket_bug.txt: Fix and unit test for the argparse inner bracket metavar bug
  • format_usage.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:

    assignee = 'https://github.com/ncoghlan'
    closed_at = <Date 2018-06-09.10:53:29.284>
    created_at = <Date 2011-04-19.08:20:50.571>
    labels = ['3.7', 'type-bug', 'library']
    title = 'argparse assertion failure with brackets in metavars'
    updated_at = <Date 2018-06-09.10:53:29.283>
    user = 'https://bugs.python.org/htnieman'

    bugs.python.org fields:

    activity = <Date 2018-06-09.10:53:29.283>
    actor = 'ncoghlan'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2018-06-09.10:53:29.284>
    closer = 'ncoghlan'
    components = ['Library (Lib)']
    creation = <Date 2011-04-19.08:20:50.571>
    creator = 'htnieman'
    dependencies = []
    files = ['21832', '30911', '30913', '30915', '35028']
    hgrepos = []
    issue_num = 11874
    keywords = ['patch']
    message_count = 23.0
    messages = ['134021', '134558', '134741', '149547', '193021', '193022', '193042', '193046', '193068', '193072', '193112', '193187', '216812', '230187', '230198', '262105', '294575', '295214', '319041', '319050', '319054', '319124', '319127']
    nosy_count = 15.0
    nosy_names = ['ncoghlan', 'bethard', 'eric.araujo', 'ysj.ray', 'xuanji', 'htnieman', 'manveru', 'paul.j3', 'wim.glenn', 'vajrasky', 'wolma', 'mZarjk', 'spaceone', 'mattpr', 'miss-islington']
    pr_nums = ['1826', '7527', '7528', '7530', '7553']
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue11874'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7']

    @htnieman
    Copy link
    Mannequin Author

    htnieman mannequin commented Apr 19, 2011

    I run this script with option --help
    -----8<------------------

    import argparse
    parser = argparse.ArgumentParser()
    parser.add_argument ('--from-itm', dest="from_itm",action="store",default=None,help ="Source ITM file", metavar="ITMFILENAME")
    parser.add_argument ("--from-fbm-sigtab",dest="from_sigtab",action="store",default=None,help="Source signal FB", metavar=" [LIB,]FBNAME")
    parser.add_argument ("--c",dest="f",metavar="x]d")
    args = parser.parse_args()
    ---------8<

    and I get an assertion failure:
    ---------8<--------------------

    D:\temp>argparsebug.py --help
    Traceback (most recent call last):
      File "D:\temp\argparsebug.py", line 6, in <module>
        args = parser.parse_args()
      File "C:\Python27\lib\argparse.py", line 1678, in parse_args
        args, argv = self.parse_known_args(args, namespace)
      File "C:\Python27\lib\argparse.py", line 1710, in parse_known_args
        namespace, args = self._parse_known_args(args, namespace)
      File "C:\Python27\lib\argparse.py", line 1916, in _parse_known_args
        start_index = consume_optional(start_index)
      File "C:\Python27\lib\argparse.py", line 1856, in consume_optional
        take_action(action, args, option_string)
      File "C:\Python27\lib\argparse.py", line 1784, in take_action
        action(self, namespace, argument_values, option_string)
      File "C:\Python27\lib\argparse.py", line 993, in __call__
        parser.print_help()
      File "C:\Python27\lib\argparse.py", line 2303, in print_help
        self._print_message(self.format_help(), file)
      File "C:\Python27\lib\argparse.py", line 2277, in format_help
        return formatter.format_help()
      File "C:\Python27\lib\argparse.py", line 278, in format_help
        help = self._root_section.format_help()
      File "C:\Python27\lib\argparse.py", line 208, in format_help
        func(*args)
      File "C:\Python27\lib\argparse.py", line 329, in _format_usage
        assert ' '.join(opt_parts) == opt_usage
    AssertionError
    --------------8<

    It must be the combination of several argument lines because when I
    comment out one of the add_arguments, it works.

    Is this a bug or do I do things I shouln't?
    If it is not allowed to use [] in metavars (what I wanted was metavar='[optional,]mantatory'), I' like to receive a readable error message.

    With best regards
    Hartmut Niemann

    @htnieman htnieman mannequin added the type-bug An unexpected behavior, bug, or error label Apr 19, 2011
    @manveru
    Copy link
    Mannequin

    manveru mannequin commented Apr 27, 2011

    I was a victim of the same issue, but only after my usage list does not fit in one line. Please fix!

    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Apr 29, 2011

    Seem as a problem in optparse.HelpFormatter._format_usage(): when the generated usage string is too long(longer than 78, e.g.), python tries to break the usage string into parts at some proper positions and group them to multiple lines, then join the parts with space and assert it equal with original usage string. The regular expression used to break the usage string into parts is:
    """r'\(.*?\)+|\[.*?\]+|\S+'"""
    So when there is '[]' in usage string, like in the example(there is '[]' in metavar), the assertion fails.

    metavar="[LIB,]FBNAME" seems quite reasonable and usable, in the case that one want to define an option who has tow values and the first is optional. So I suggest '[]' should be allowed in metavar, and the _format_usage() needs fixed.

    Here is a patch that fixes by changing the way breaking the usage string to parts and remove joining the parts and assert it equal to original usage string. Now the usage string is broken into intact each action of group usage strings. I think this breaking granularity is enough.

    @ysjray ysjray mannequin added the stdlib Python modules in the Lib dir label Apr 29, 2011
    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Dec 15, 2011

    I agree this is a bug. The patch needs to add unit tests that make sure metavars with [] work as expected.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Jul 14, 2013

    Attached the unit test.

    Here is the simpler script to product the bug:

    ------------------------

    import argparse
    parser = argparse.ArgumentParser(prog='PROG')
    parser.add_argument ('--a', metavar='a'*76)
    parser.add_argument ('--b', metavar="[innerpart]outerpart")
    parser.add_argument ('c', metavar='c'*76)
    parser.add_argument ('d', metavar="[innerpart2]outerpart2")
    args = parser.parse_args()

    python thefile.py --help

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Jul 14, 2013

    Sorry, got typo in last unit test.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Jul 14, 2013

    Tidied up the unit test. Mixed with arguments without metavar.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Jul 14, 2013

    Attached the preliminary fix and the unit test for this ticket.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 14, 2013

    If the arg_parts are passed through the same cleanup as the 'text' (and empty strings removed), then

    text = ' '.join(arg_parts)
    

    In that case there would be no need to return both (text, arg_parts).

    Parenthesis in the metavar could also create the problem addressed in this thread, except as noted in http://bugs.python.org/issue18349 that 'text' cleanup removes them.

    nargs='*' or '+' or integer is another way in which [] could be introduced into the metavar.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 15, 2013

    I just filed a patch with http://bugs.python.org/issue10984 (argparse add_mutually_exclusive_group should accept existing arguments to register conflicts) that includes the latest patch from this issue. I tweaked it so _format_actions_usage only returns arg_parts. The block of _re. statements (# clean up separators for mutually exclusive groups) are in a nested function so it can be applied to each of the parts.

    In that issue I wrote a custom formatter that handles groups even if they share actions, or the action order does not match definition order. For that it is easier to work with the arg_parts as generated here rather than the whole usage line.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Jul 15, 2013

    paul j3, thanks for reviewing my patch and giving me credit in your patch for another ticket.

    Yeah, as you could see, the reason I return arg_parts and text is because the text still needs to undergo the cleanup process. You solved it by putting cleaning up in inner function.

    I am thinking whether it is best to do "assert ' '.join(opt_parts) == opt_usage" inside _format_actions_usage helper function.

    In that way, we can avoid returning the text. We can return only the arg_parts.

    Anyway, my patch still got some unused variables, notably part_regexp and inner. My bad.

    Let me check the code more deeply. See whether I can architect my patch in a better way. Maybe we can avoid building separate list inside _format_actions_usage.

    Beside of that, this bug is not introduced solely by bracket character. It needs another non-space character on the right side of it.

    This line is fine:
    parser.add_argument ('--b', metavar="[innerpart] outerpart")

    This line will fail the assertion:
    parser.add_argument ('--b', metavar="[innerpart]outerpart")

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 16, 2013

    Here's a patch that takes a different approach - rewrite _format_actions_usage() so it formats groups (and unattached actions) directly.

    It uses the same logic to determine when to format a group, but then it calls _format_group_usage() to format the group, without the error prone insert and cleanup business. And by keeping a list of the parts, it avoids the complications of parsing the text. The only cleanup regex removes () from single element groups.

    In the nested get_lines function, I added 'line and' to

       if line and line_len + 1 + len(part) > text_width:

    so it does not create a blank line before an extra long entry (e.g. 'c'*76).

    I'm using the _format_group_usage() and _format_just_actions_usage() in the bpo-10984 MultiGroupFormatter, just rearranging the logic in its _format_actions_usage() method. That patch could be rewritten to depend on this one.

    This patch also fixes http://bugs.python.org/issue18349, since the metavar is never parsed.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Apr 18, 2014

    Another example of code hitting this AssertionError. Here the problem was a space in the option argument, '--out '.

    http://stackoverflow.com/questions/23159845/python-argparse-assertionerror

    'parser.add_argument('-o', '--out ', help='b', required = True)'

    That space would have cause problems later when trying access the 'args' attributes. But producing the error during the help formatting didn't help.

    @wm75
    Copy link
    Mannequin

    wm75 mannequin commented Oct 28, 2014

    It doesn't seem to be the exact same problem, but still this very simple example with parentheses in metavar fails to format correctly:

    import argparse
    parser = argparse.ArgumentParser()
    parser.add_argument("inputfiles", metavar = 'input_file(s)', nargs = "+", help = 'one or more input files')
    
    args = parser.parse_args()

    -->

    usage: -m [-h] input_files) [input_file(s ...]

    positional arguments:
    input_file(s) one or more input files

    optional arguments:
    -h, --help show this help message and exit

    with the two occurences of input_file(s) being formatted wrong, but differently.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Oct 29, 2014

    The patch with a major rewrite of '_format_actions_usage' handles this usage problem correctly:

    usage: try_para_in_meta.py [-h] input_file(s) [input_file(s) ...]
    

    The error is the result of usage group formatter trying to remove excess (). To give an idea of what is happening I replaced the missing () with {}.

    input_file(s) [input_file(s) ...]
    input_file{s) [input_file(s} ...]
    

    @mattpr
    Copy link
    Mannequin

    mattpr mannequin commented Mar 21, 2016

    Same AssertionError is also caused by having certain "choices".

    Python 2.7.10

    global_options.add_argument('--field-sep', choices=[',',';','|','\t'], required=True, help='Field separator that separates columns in a row.')

    Removing required=True or removing the tab (\t) from the options both work around this usage formatter issue for me.

    I know this is an old issue but figured I would add another repro case to help whoever might work on this.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented May 27, 2017

    Any changes here including the latest pull request might conflict with

    http://bugs.python.org/issue29553

    Argparser does not display closing parentheses in nested mutex groups

    If someone uses nested mutually exclusive groups (which I don't think they should, but anyways ...), the usage formatter gets confused by the nested brackets, and weeds out the wrong one(s).

    That issue hasn't been settle yet; my feeling is that fixing this assertion error properly is more important.

    @wimglenn
    Copy link
    Mannequin

    wimglenn mannequin commented Jun 5, 2017

    May I ask, how to assign reviewers for CPython pull requests? #1826 has been sitting there for 10 days and the only comment was from a bot.

    @wimglenn wimglenn mannequin added the 3.7 (EOL) end of life label Sep 14, 2017
    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Jun 8, 2018

    New changeset 66f02aa by Nick Coghlan (wim glenn) in branch 'master':
    bpo-11874: fix assertion failure in argparse metavar handling (GH-1826)
    66f02aa

    @miss-islington
    Copy link
    Contributor

    New changeset 376c272 by Miss Islington (bot) in branch '3.6':
    bpo-11874: fix assertion failure in argparse metavar handling (GH-1826)
    376c272

    @miss-islington
    Copy link
    Contributor

    New changeset 842985f by Miss Islington (bot) in branch '3.7':
    bpo-11874: fix assertion failure in argparse metavar handling (GH-1826)
    842985f

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Jun 9, 2018

    Thanks for the PR, and your patience!

    The change has now been merged for all active 3.x versions.

    It turns out the 2.7 argparse code is still close enough to the 3.x code for the automated backport to work, so the CI for that is currently running.

    @ncoghlan ncoghlan self-assigned this Jun 9, 2018
    @miss-islington
    Copy link
    Contributor

    New changeset 4e6bd24 by Miss Islington (bot) in branch '2.7':
    bpo-11874: fix assertion failure in argparse metavar handling (GH-1826)
    4e6bd24

    @ncoghlan ncoghlan closed this as completed Jun 9, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants