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: arg groups and mutually exclusive groups behave inconsitently #90216

Closed
LA-Toth mannequin opened this issue Dec 12, 2021 · 4 comments
Closed

argparse: arg groups and mutually exclusive groups behave inconsitently #90216

LA-Toth mannequin opened this issue Dec 12, 2021 · 4 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@LA-Toth
Copy link
Mannequin

LA-Toth mannequin commented Dec 12, 2021

BPO 46058
Nosy @LA-Toth, @iritkatriel
Superseder
  • bpo-22047: Deprecate unsupported nesting of argparse groups
  • 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:45:51.983>
    created_at = <Date 2021-12-12.19:01:14.265>
    labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
    title = 'argparse: arg groups and mutually exclusive groups behave inconsitently'
    updated_at = <Date 2022-01-02.01:52:28.636>
    user = 'https://github.com/LA-Toth'

    bugs.python.org fields:

    activity = <Date 2022-01-02.01:52:28.636>
    actor = 'paul.j3'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-12-16.15:45:51.983>
    closer = 'iritkatriel'
    components = ['Library (Lib)']
    creation = <Date 2021-12-12.19:01:14.265>
    creator = 'Laszlo.Attila.Toth'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46058
    keywords = []
    message_count = 4.0
    messages = ['408405', '408407', '408719', '409484']
    nosy_count = 3.0
    nosy_names = ['paul.j3', 'Laszlo.Attila.Toth', 'iritkatriel']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '22047'
    type = 'behavior'
    url = 'https://bugs.python.org/issue46058'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @LA-Toth
    Copy link
    Mannequin Author

    LA-Toth mannequin commented Dec 12, 2021

    I tried to add arguments to process DB-related settings, either from typing import Optional
    from a file
    or explicitly specifying them. In theory with nested groups (by add_argument_group
    and add_mutually_exlusive_group) this can be implemented in almost straightforward way:

    # test.py
    import argparse
    
    parser = argparse.ArgumentParser()
    dbsettings = parser.add_argument_group('Database settings')
    xdbgrp = dbsettings.add_mutually_exclusive_group(required=True)
    xdbgrp.add_argument('--db-config')
    
    grp = xdbgrp.add_argument_group(required=True)
    grp.add_argument('--db-host')
    grp.add_argument('--db-user')
    
    xgrp = grp.add_mutually_exclusive_group()
    xgrp.add_argument('--db-password')
    xgrp.add_argument('--db-password-file')
    parser.parse_args()

    But there are issues:

    1. the add_mutually_exclusive_group has only one optional parameter, required=False by default,
      so I cannot provide a title, I have to create yet another group (xdbgrp in the example)

    2. I would expect the parser do the complete argument parsing and validation, so I don't
      need to implement certain steps. In this example I expect to have a --db-host arg
      if the --db-config is not specified. But if I add required=True, the argparse
      module expects that with --db-config the --db-host is also specified.
      In other words the xdbgrp mutually exclusive group fails to be mutually exclusive.

    3. While xgrp behaves correctly, I cannot specify both --db-password and --db-password-file,
      I still can specify them with --db-config (see Rename README to README.rst and enhance formatting #2)

    4. If I run it as: python3 test.py --db-host x
      the command fails:
      usage: test.py [-h] --db-config DB_CONFIG --db-host DB_HOST [--db-user DB_USER]
      [--db-password DB_PASSWORD | --db-password-file DB_PASSWORD_FILE]
      test.py: error: one of the arguments --db-config is required

      So both --db-config and --db-host are required, the embedded group, grp fails to work,
      or prehaps again the xdbgrp fails (depends on the view)

    5. Removing all required=True options the following is accepted:
      python3 test.py --db-host x --db-config y
      so the xdbgrp mutually exclusive group again doesn't work.

    6. Only xdbgrp is required, --db-host is not:
      python3 test.py --db-host x
      usage: test.py [-h] --db-config DB_CONFIG [--db-host DB_HOST] [--db-user DB_USER]
      [--db-password DB_PASSWORD | --db-password-file DB_PASSWORD_FILE]
      test.py: error: one of the arguments --db-config is required
      Again, the group embedded into a mutually exclusive group is not handled correctly

    What is expected:

    1. add_mutually_exclusive_group can have title/description, but unfortunately it is not
      backward compatible

    2. If I add a mutally exclusive group, it has XOR relation between its arguments and
      argument groups.

    3. An argument group is handled as a single entity similar to an argument.
      Basically this is the same as Rename README to README.rst and enhance formatting #2.

    4. A required argument affects only its argument group and the parent group
      and so on till the parser, but this chain stops at a mutually exclusive group,
      based on Rename README to README.rst and enhance formatting #2 and bpo-29403: Fix mock's broken autospec behavior on method-bound builtin functions #3 .

    @LA-Toth LA-Toth mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.10 only security fixes 3.11 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Dec 12, 2021
    @LA-Toth
    Copy link
    Mannequin Author

    LA-Toth mannequin commented Dec 12, 2021

    Checking the code the add_mutually_exclusive_group has only kwargs, so one part can be fixed (POC, breaks tests):

    --- Lib/argparse.py
    +++ Lib/argparse.py
    @@ -1648,8 +1648,8 @@ def _remove_action(self, action):

     class _MutuallyExclusiveGroup(_ArgumentGroup):
    -    def __init__(self, container, required=False):
    -        super(_MutuallyExclusiveGroup, self).__init__(container)
    +    def __init__(self, container, required=False, title=None, description=None):
    +        super(_MutuallyExclusiveGroup, self).__init__(container, title, description)
             self.required = required
             self._container = container

    @@ -2529,6 +2529,14 @@ def format_help(self):
    formatter.add_arguments(action_group._group_actions)
    formatter.end_section()

    + for mutual_group in self._mutually_exclusive_groups:
    + if not mutual_group.title:
    + continue
    + formatter.start_section(mutual_group.title)
    + formatter.add_text(mutual_group.description)
    + formatter.add_arguments(mutual_group._group_actions)
    + formatter.end_section()
    +
    # epilog
    formatter.add_text(self.epilog)

    @iritkatriel iritkatriel removed 3.7 (EOL) end of life 3.8 only security fixes labels Dec 12, 2021
    @iritkatriel
    Copy link
    Member

    Nesting argument groups and mutually exclusive groups is now deprecated (see bpo-22047). Thank you for the bug report.

    Note Paul's comment about why nesting mutually exclusive groups does not give you anything in terms of semantics.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jan 2, 2022

    At least until these latest deprecations, the only nesting that made sense was to put a mutually_exclusive_group inside an argument_group. This was a way of providing a title and description for the exclusive_group. (And not documented.) I don't know if that's still possible.

    'required' only makes sense for the exclusive_group. I don't know what happens when one tries to give it to an argument_group. If it doesn't raise an error, I expect it to be ignored.

    argument_groups are only used for help formatting; they have no role in parsing. exclusive_groups are primarily a parsing checking tool. Usage formatting tries to display exclusive groups, but is easily broken.

    While mutually_exclusive_group is a subclass of argument_group (and that in turn a subclass of argument_container), very little usage or behavior is inherited. Don't expect any consistency.

    A key point, that is easily lost, is that all groups share the _actions list with the parser. When an argument is added a group (either kind), it is, in effect, added to the parser's _actions list. So when parsing, there's only one list of Actions.

    A group will also keep the Action in its own _group_actions list, which is used for formatting or for exclusive checking. But otherwise the _group_actions list not used for parsing.

    Another point, is that there are 2 default argument_groups. Thus every Action is in an _group_actions list. If an exclusive_group is not nested in an argument_group, its Actions will be added to one of the defaults (optionals or positionals).

    @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.9 only security fixes 3.10 only security fixes 3.11 only 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

    1 participant