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

Deprecate unsupported nesting of argparse groups #66246

Closed
SamKerr mannequin opened this issue Jul 23, 2014 · 18 comments
Closed

Deprecate unsupported nesting of argparse groups #66246

SamKerr mannequin opened this issue Jul 23, 2014 · 18 comments
Labels
3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@SamKerr
Copy link
Mannequin

SamKerr mannequin commented Jul 23, 2014

BPO 22047
Nosy @rhettinger, @LA-Toth, @serhiy-storchaka, @iritkatriel
PRs
  • bpo-22047: [argparse] deprecate nested argument groups and mutually e… #30098
  • Files
  • issue22047_1.patch
  • issue22047.py
  • issue22047_2.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 = None
    closed_at = <Date 2021-12-16.15:34:57.014>
    created_at = <Date 2014-07-23.16:03:44.124>
    labels = ['type-bug', 'library', '3.11']
    title = 'Deprecate unsupported nesting of argparse groups'
    updated_at = <Date 2021-12-17.14:29:50.005>
    user = 'https://bugs.python.org/SamKerr'

    bugs.python.org fields:

    activity = <Date 2021-12-17.14:29:50.005>
    actor = 'iritkatriel'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-12-16.15:34:57.014>
    closer = 'iritkatriel'
    components = ['Library (Lib)']
    creation = <Date 2014-07-23.16:03:44.124>
    creator = 'Sam.Kerr'
    dependencies = []
    files = ['36093', '36095', '36100']
    hgrepos = []
    issue_num = 22047
    keywords = ['patch']
    message_count = 16.0
    messages = ['223744', '223917', '223972', '223974', '223978', '224004', '224010', '224020', '286584', '408220', '408412', '408524', '408637', '408712', '408765', '408782']
    nosy_count = 6.0
    nosy_names = ['rhettinger', 'paul.j3', 'Laszlo.Attila.Toth', 'serhiy.storchaka', 'Sam.Kerr', 'iritkatriel']
    pr_nums = ['30098']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22047'
    versions = ['Python 3.11']

    @SamKerr
    Copy link
    Mannequin Author

    SamKerr mannequin commented Jul 23, 2014

    The following code:

       import argparse
       parser = argparse.ArgumentParser()
       group1 = parser.add_mutually_exclusive_group()
       group2 = group1.add_mutually_exclusive_group()
       group2.add_argument('-hello',action='store_true', help="A flag")
       args = parser.parse_args()

    produces this output:

    skerr@gravel:~$ python bug.py -h
    usage: bug.py [-h] [[-hello]

    optional arguments:
    -h, --help show this help message and exit
    -hello A flag
    skerr@gravel:~$

    Note the double [[ around hello, but there is no double ]] to close it. This is the error.

    @SamKerr SamKerr mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jul 23, 2014
    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 25, 2014

    That's an artifact of how the group usage is formatted (which isn't very robust). It's not designed to handle nested groups.

    Mutually exclusive groups aren't meant to nest inside other mutually exclusive groups. While it possible, it doesn't help you. Effectively the arguments combined into one larger group.

    See http://bugs.python.org/issue11588 for discussion on how nested groups might be implemented, and the difficulties in properly formatting their usage.

    http://bugs.python.org/issue10984 has a more robust mutually exclusive group formatter, but even that is not designed for nesting.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 25, 2014

    On further thought, I think

       group2 = group1.add_mutually_exclusive_group()

    should have raised an error, stating that a group (argument or mutually exclusive) cannot be added to a mutually exclusive group.

    Also an argument group should not be added to another argument group.

    However, adding a mutually exclusive group to an argument group is ok, though only for the undocumented (though tested) purpose of giving it a title.

    @SamKerr
    Copy link
    Mannequin Author

    SamKerr mannequin commented Jul 25, 2014

    What I was going for was the ability to have a group contain a mutually exclusive group and a non-exclusive group, but using the mutually exclusive group meant you could not use the non-exclusive group.

    Such as:

    [ [ -opt1 | -opt2 | -opt3 ] [ [-opt4] [-opt5] [-opt6] ] ]

    @SamKerr
    Copy link
    Mannequin Author

    SamKerr mannequin commented Jul 25, 2014

    I fat fingered the example, sorry:

    [ [ -opt1 | -opt2 | -opt3 ] | [ [-opt4] [-opt5] [-opt6] ] ]

    Note the new pipe between the 2 subgroups

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 25, 2014

    Here's a preliminary patch that raises an error if there's an attempt to nest a mutually exclusive group in another, or there's an attempt to add an argument group to either kind of group.

    It still needs test_argparse.py and argparse.rst changes

    I'm raising a ValueError, since that is what most of the other add_argument errors do. An alternative is a NotImplementedError, since that is, in effect, what I am doing, blocking the implementation of particular 'add' methods.

    An alternative to adding this patch as high priority bug issue, is to include it in the UsageGroup patch (11588) which will implement nestable groups.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 25, 2014

    ArgumentGroups and MutuallyExclusiveGroups, as currently defined, won't give you that kind of usage. I have appended a script that uses UsageGroups, which I am developing for http://bugs.python.org/issue11588,
    to solve this.

    It defines 2 'mxg' groups (groups with the xor logic of mutually exclusive groups), and 1 'any' group. They can be nested.

    The resulting usage line is:

    usage: PROG [-h] [[--opt1 | --opt2 | --opt3] | [--opt4 --opt5 --opt6]]
    

    Normally '|' is used for simple logical 'or'. But in mutually exclusive groups it denotes 'xor'. So what should join 'any' lists? You chose ' ', I was using ','. Defining a usage notation that is simple, intuitive, and also flexible, is not easy.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 26, 2014

    This patch adds a

    class TestMutuallyExclusiveGroupErrors
        test_invalid_add_group() test,
    

    closely modeled on

            test_invalid_add_argument()

    I'm using ValueError in group add methods (maintaining the similarity with add_argument errors).

    I haven't changed the documentation. add_argument_group and add_mutually_exclusive_group methods are described as belonging to an ArgumentParser, and the examples are consistent with that. An admonition against nesting groups would not fit with the current flow.

    However to be accurate, these methods belong to _ActionsContainer, the parent class for both the parser and groups. The documentation glosses over this detail. So an alternative way of addressing this issue is to move these 2 methods to the ArgumentParser class.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Feb 1, 2017

    A similar issue about nesting an Argument Group inside a Mutually Exclusive Group.

    http://bugs.python.org/issue24736

    @iritkatriel
    Copy link
    Member

    I am unable to reproduce this on 3.11:

    % ./python.exe tt.py -h
    usage: tt.py [-h] [[-hello]]

    options:
    -h, --help show this help message and exit
    -hello A flag

    @iritkatriel
    Copy link
    Member

    While I was unable to reproduce this rendering error, there are other issues due to nesting of argument groups, and I wonder if we should deprecate those operations, along the lines of Paul's patch on this issue (but with deprecation rather than raising an exception).

    Other related issues: bpo-46057 (from today), bpo-16807, bpo-45690, bpo-43259, (there are probably more).

    @iritkatriel
    Copy link
    Member

    I am repurposing this issue for the deprecation of nesting. It is pretty clear that the functions exist by accident through inheritance. They are not documented and from the number of open issues about them they clearly are not working as people expect them to.

    @iritkatriel iritkatriel added the 3.11 bug and security fixes label Dec 14, 2021
    @iritkatriel iritkatriel changed the title argparse improperly prints mutually exclusive options when they are in a group Deprecate unsupported nesting of argparse groups Dec 14, 2021
    @iritkatriel
    Copy link
    Member

    Another issue due to nesting: bpo-38590.

    @iritkatriel
    Copy link
    Member

    New changeset 30322c4 by Irit Katriel in branch 'main':
    bpo-22047: [argparse] deprecate nested argument groups and mutually exclusive groups (GH-30098)
    30322c4

    @LA-Toth
    Copy link
    Mannequin

    LA-Toth mannequin commented Dec 17, 2021

    Hi, according to the group update the _MutuallyExclusiveGroup should have an add_argument_group() call with the deprecation warning, but that method is missing in commit 30322c4.

    @iritkatriel
    Copy link
    Member

    _MutuallyExclusiveGroup inherits add_argument_group from _ArgumentGroup.

    @scottamain
    Copy link
    Contributor

    Hi @iritkatriel, based on your change 30322c4, it appears that it's still okay to call add_mutually_exclusive_group() on a regular argument group (such as to provide a title and description for the mutually exclusive group). Can you please confirm that's indeed intended to work?

    @iritkatriel
    Copy link
    Member

    Yes, this combination is useful and it works, so it is supported.

    ruffsl added a commit to ruffsl/colcon-cache that referenced this issue Nov 24, 2022
    to avoid DeprecationWarning: Nesting argument groups is deprecated.
    python/cpython#66246
    ruffsl added a commit to ruffsl/colcon-cache that referenced this issue Dec 6, 2022
    * Comment out added argument groups
    to avoid DeprecationWarning: Nesting argument groups is deprecated.
    python/cpython#66246
    
    * Swap to TODOs and fix linecount
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 bug and security fixes 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