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 fails with required subparsers, un-named dest, and empty argv #73484

Closed
zpincus mannequin opened this issue Jan 17, 2017 · 9 comments
Closed

argparse fails with required subparsers, un-named dest, and empty argv #73484

zpincus mannequin opened this issue Jan 17, 2017 · 9 comments
Assignees
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

@zpincus
Copy link
Mannequin

zpincus mannequin commented Jan 17, 2017

BPO 29298
Nosy @merwok, @zpincus, @encukou, @paulie4, @hroncok, @miss-islington, @Lucas-C, @greg-minshall, @iritkatriel
PRs
  • bpo-29298: Fix crash with required subparsers without dest #3680
  • bpo-29298: Fix crash for required subparsers in argparse #18564
  • bpo-29298: Fixing argparse required subparsers error handling #26278
  • [3.10] bpo-29298: Fix crash with required subparsers without dest (GH-3680) #27303
  • [3.9] bpo-29298: Fix crash with required subparsers without dest (GH-3680) #27304
  • 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/merwok'
    closed_at = <Date 2021-07-27.15:42:54.068>
    created_at = <Date 2017-01-17.14:52:49.868>
    labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
    title = 'argparse fails with required subparsers, un-named dest, and empty argv'
    updated_at = <Date 2021-07-27.15:42:54.067>
    user = 'https://github.com/zpincus'

    bugs.python.org fields:

    activity = <Date 2021-07-27.15:42:54.067>
    actor = 'petr.viktorin'
    assignee = 'eric.araujo'
    closed = True
    closed_date = <Date 2021-07-27.15:42:54.068>
    closer = 'petr.viktorin'
    components = ['Library (Lib)']
    creation = <Date 2017-01-17.14:52:49.868>
    creator = 'zachrahan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29298
    keywords = ['patch']
    message_count = 9.0
    messages = ['285646', '285877', '330133', '357839', '393958', '396654', '398049', '398051', '398052']
    nosy_count = 11.0
    nosy_names = ['eric.araujo', 'zachrahan', 'petr.viktorin', 'paul.j3', 'paulie4', 'hroncok', 'miss-islington', 'Mathias Ettinger', 'Lucas Cimon', 'Minshall', 'iritkatriel']
    pr_nums = ['3680', '18564', '26278', '27303', '27304']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29298'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @zpincus
    Copy link
    Mannequin Author

    zpincus mannequin commented Jan 17, 2017

    In python 3.6 (and several versions back), using argparse with required subparsers will cause an unhelpful TypeError if the 'dest' parameter is not explicitly specified, and no arguments are provided.

    Test case:
    import argparse
    parser = argparse.ArgumentParser()
    subparsers = parser.add_subparsers()
    subparsers.required = True
    args = parser.parse_args([])

    Observed result:
    TypeError: sequence item 0: expected str instance, NoneType found

    If the line above is changed to:
    subparsers = parser.add_subparsers(dest='function')

    Then the following is printed to stderr:
    usage: python [-h] {} ...
    python: error: the following arguments are required: function

    This issue goes back at least several years:
    http://stackoverflow.com/questions/23349349/argparse-with-required-subparser/23354355

    Though it seems odd to not specify a dest in the add_subparsers line, the pattern is not completely useless. The below works fine without setting a 'dest' in add_subparsers, except when argv is empty:
    sub1 = subparsers.add_parser('print')
    sub1.set_defaults(function=print)

    However, an empty argv produces the unexpected TypeError above. I'm not sure if argparse should provide a more useful exception in this case, or if there is a clean way to do the right thing without a dest specified.

    @zpincus zpincus mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Jan 17, 2017
    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jan 20, 2017

    http://bugs.python.org/issue9253 argparse: optional subparsers

    Initially this bug/issue was a request to allow subparsers to be optional. But with the change in how required actions are handled, subparsers are now optional by default.

    As you learned from the SO question you now have to specify

    subparsers.required = True

    This is also discussed in my post (and following ones)

    http://bugs.python.org/issue9253#msg186387

    The default 'dest' is SUPPRESS. The error you report occurs because the 'required' error mechanism cannot handle that value. The suggest fix is to assign dest, even if it is not needed in the Namespace. For now it is needed for error reporting.

    Reviewing my suggested patches, it looks like I generate a 'dest' substitute from the subparser names. So the 'required' error would look like

    python: error: the following arguments are required: {cmd1, cmd2}

    I think this issue can be closed with a reference to 9253. Or maybe that issue is too old, long and confusing, and we need a new bug/issue.

    @merwok merwok added the 3.7 (EOL) end of life label Sep 20, 2017
    @merwok merwok self-assigned this Sep 20, 2017
    @MathiasEttinger
    Copy link
    Mannequin

    MathiasEttinger mannequin commented Nov 20, 2018

    I was just hit by the very same issue and added the following test into _get_action_name to work around it:

    elif isinstance(argument, _SubParsersAction):
        return '{%s}' % ','.join(map(str, argument.choices))
    

    I checked bpo-9253 as referenced by paul j3 and like the argument.name() approach as well as it's less specific.

    Any chance this can be addressed one way or another?

    @greg-minshall
    Copy link
    Mannequin

    greg-minshall mannequin commented Dec 5, 2019

    while waiting for a fix, would it be possible to document in the argparse documentation that the 'dest' parameter is required (at least temporarily) for add_subparsers()? (somewhere near file:///usr/share/doc/python/html/library/argparse.html#sub-commands)

    gratuitous diff: the pull request from 2017 would probably fix it. my diffs are here (from: Python 3.8.0 (default, Oct 23 2019, 18:51:26). (the pull request changes the utility '_get_action_name'; i wasn't sure of side-effects with other callers, so changed nearer the failure location.)
    ----
    *** new/argparse.py 2019-12-05 11:16:37.618985247 +0530
    --- old/argparse.py 2019-10-24 00:21:26.000000000 +0530


    *** 2017,2030 ****
    for action in self._actions:
    if action not in seen_actions:
    if action.required:
    ! ra = _get_action_name(action)
    ! if ra is None:
    ! if not action.choices == {}:
    ! choice_strs = [str(choice) for choice in action.choices]
    ! ra = '{%s}' % ','.join(choice_strs)
    ! else:
    ! ra = '<unknown>'
    ! required_actions.append(ra)
    else:
    # Convert action default now instead of doing it before
    # parsing arguments to avoid calling convert functions
    --- 2017,2023 ----

              for action in self._actions:
                  if action not in seen_actions:
                      if action.required:
    !                     required_actions.append(_get_action_name(action))
                      else:
                          # Convert action default now instead of doing it before
                          # parsing arguments to avoid calling convert functions

    @BenSokol BenSokol mannequin added 3.8 only security fixes 3.9 only security fixes type-crash A hard crash of the interpreter, possibly with a core dump and removed type-bug An unexpected behavior, bug, or error labels Feb 20, 2020
    @iritkatriel
    Copy link
    Member

    crash means segfault, not unhandled exception.

    @iritkatriel iritkatriel added 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error and removed 3.7 (EOL) end of life 3.8 only security fixes type-crash A hard crash of the interpreter, possibly with a core dump labels May 19, 2021
    @paulie4
    Copy link
    Mannequin

    paulie4 mannequin commented Jun 28, 2021

    I'd like to second this idea, since it's very confusing without it:

    while waiting for a fix, would it be possible to document in the argparse documentation that the 'dest' parameter is required (at least temporarily) for add_subparsers()? (somewhere near file:///usr/share/doc/python/html/library/argparse.html#sub-commands)

    @miss-islington
    Copy link
    Contributor

    New changeset 17575f7 by Anthony Sottile in branch 'main':
    bpo-29298: Fix crash with required subparsers without dest (GH-3680)
    17575f7

    @encukou
    Copy link
    Member

    encukou commented Jul 23, 2021

    New changeset c589992 by Miss Islington (bot) in branch '3.10':
    bpo-29298: Fix crash with required subparsers without dest (GH-3680) (GH-27303)
    c589992

    @encukou
    Copy link
    Member

    encukou commented Jul 23, 2021

    New changeset 0978018 by Miss Islington (bot) in branch '3.9':
    bpo-29298: Fix crash with required subparsers without dest (GH-3680) (GH-27304)
    0978018

    @encukou encukou closed this as completed Jul 27, 2021
    @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

    4 participants