python / cpython

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

argparse group nesting lost on inheritance #61011

Closed e1d29b37-30c5-4023-830b-f8e2f5354e80 closed 4 weeks ago

e1d29b37-30c5-4023-830b-f8e2f5354e80 commented 11 years ago
BPO 16807
Nosy @dougalsutherland, @iritkatriel
Files
  • argparse_mutex_parent.patch: patch fixing the problem and adding a test
  • 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 group nesting lost on inheritance' updated_at = user = 'https://github.com/dougalsutherland' ``` bugs.python.org fields: ```python activity = actor = 'iritkatriel' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'dougalsutherland' dependencies = [] files = ['28473'] hgrepos = [] issue_num = 16807 keywords = ['patch'] message_count = 5.0 messages = ['178459', '222934', '223047', '223158', '408222'] nosy_count = 4.0 nosy_names = ['bethard', 'paul.j3', 'dougalsutherland', 'iritkatriel'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue16807' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    Linked PRs

    e1d29b37-30c5-4023-830b-f8e2f5354e80 commented 11 years ago

    If you wrap a mutually exclusive group inside an argument group in an argparse.ArgumentParser, and then use parents= to inherit from that parser, the non-exclusive argument group is forgotten in the help output, and the arguments move to the default "optional arguments" section. For example:

        # construct the parser with a mutually exclusive group
        >>> import argparse
        >>> parent = argparse.ArgumentParser(add_help=False)
        >>> group = parent.add_argument_group('the group')
        >>> group.add_argument('--foo') and None
        >>> mutex = group.add_mutually_exclusive_group()
        >>> mutex.add_argument('-a', action='store_true') and None
        >>> mutex.add_argument('-b', action='store_true') and None
        >>> parent.print_help()
        usage: [--foo FOO] [-a | -b]
    
        the group:
          --foo FOO
          -a
          -b
        # now try to inherit from it; "the group" is forgotten for
        # mutex arguments, but remains for others
        >>> argparse.ArgumentParser(add_help=False, parents=[parent]).print_help()
        usage: [--foo FOO] [-a | -b]
    
        optional arguments:
          -a
          -b
    
        the group:
          --foo FOO

    I see the same behavior on 2.7.3 and 3.3.0.

    The problem is that argparse._ActionsContainer._add_container_actions always adds mutex groups to the top level, rather than to the equivalent of their _container attribute. The attached patch fixes this, and adds a test based on the formatted output (almost identical to the test_groups_parents test).

    One thing about the patch: it assumes that the _container attribute of all the mutex groups will be either the container argument to _add_container_actions or an argument group that has been processed in group_map. If this is not the case, it'll fail with either an AttributeError or a KeyError. I don't know when this would happen, or if it's common enough that it's worth checking for more explicitly.

    83d2e70e-e599-4a04-b820-3814bbdb9bef commented 10 years ago

    @Dougal sorry about the delay in getting back to you.

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

    To put this issue in perspective:

    proposes adding these attributes to MEG. There I proposed doing so via this nested group mechanism. So that patch requires this one to work correctly with parents.

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

    The subcommands grouping mechanism proposed in http://bugs.python.org/issue9341 probably does not work with [parents] either.

    This _add_container_actions method is brittle. It has to know too much about the structure of a parser and its groups. Any change to that structure could break this [parents] mechanism.

    In a refactoring, each 'add_xxx' method would have a 'copy_xxx' companion method.

    iritkatriel commented 2 years ago

    Reproduced on 3.11:

    >>> argparse.ArgumentParser(add_help=False, parents=[parent]).print_help()
    usage: [--foo FOO] [-a | -b]
    
    options:
      -a
      -b
    
    the group:
      --foo FOO
    serhiy-storchaka commented 1 month ago

    @djsutherland, is it your patch? Are you interesting in creating a PR?

    serhiy-storchaka commented 4 weeks ago

    The page of the original author of the patch @dougalsutherland was moved to @djsutherland, but that page has different name, and I do not know if they are the same person or relatives. So I used the old name to credit the author in #125210.

    djsutherland commented 4 weeks ago

    Oh wow, this has been a while!

    I am the same person; I changed my name years ago. Please change the ACKS in the other patch to Danica J Sutherland; thanks @serhiy-storchaka!

    serhiy-storchaka commented 4 weeks ago

    Done. Thank you for your contribution, Danica, and sorry it took too long to review your patch.