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: optional subparsers #53499

Open
nvie mannequin opened this issue Jul 13, 2010 · 36 comments
Open

argparse: optional subparsers #53499

nvie mannequin opened this issue Jul 13, 2010 · 36 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@nvie
Copy link
Mannequin

nvie mannequin commented Jul 13, 2010

BPO 9253
Nosy @csernazs, @jwilk, @merwok, @bitdancer, @wking, @cjerdonek, @Julian, @asottile, @Mariatta, @johnnybubonic, @epicfaace
Dependencies
  • bpo-26510: [argparse] Add required argument to add_subparsers
  • Files
  • ed0fce615582.diff
  • check.py: Test for optional subparsers vs. "too few arguments"
  • required.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 = None
    created_at = <Date 2010-07-13.21:29:47.745>
    labels = ['3.7', 'type-feature', 'library']
    title = 'argparse: optional subparsers'
    updated_at = <Date 2020-04-03.11:22:15.235>
    user = 'https://bugs.python.org/nvie'

    bugs.python.org fields:

    activity = <Date 2020-04-03.11:22:15.235>
    actor = 'bsaner'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2010-07-13.21:29:47.745>
    creator = 'nvie'
    dependencies = ['26510']
    files = ['24217', '28778', '29793']
    hgrepos = ['68', '69', '100', '101', '102']
    issue_num = 9253
    keywords = ['patch']
    message_count = 36.0
    messages = ['110231', '110232', '110244', '110309', '113267', '113512', '113558', '113575', '113577', '121250', '121271', '144484', '144512', '149533', '150757', '150784', '150816', '150821', '150953', '153771', '170491', '180229', '181855', '186052', '186387', '186532', '186695', '193956', '215894', '217461', '267692', '300283', '302294', '302296', '302661', '304594']
    nosy_count = 26.0
    nosy_names = ['csernazs', 'bethard', 'jwilk', 'frispete', 'eric.araujo', 'r.david.murray', 'zzzeek', 'labrat', 'chris.jerdonek', 'nvie', 'DasIch', 'elsdoerfer', 'G2P', 'Julian', 'dsully', 'derks', 'seblu', 'paul.j3', 'bewest', 'bkabrda', 'couplewavylines', 'svilgelm', 'Anthony Sottile', 'Mariatta', 'bsaner', 'epicfaace']
    pr_nums = []
    priority = 'high'
    resolution = None
    stage = 'test needed'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue9253'
    versions = ['Python 3.7']

    @nvie
    Copy link
    Mannequin Author

    nvie mannequin commented Jul 13, 2010

    **NOTE**: This is a re-post of http://code.google.com/p/argparse/issues/detail?id=47

    What steps will reproduce the problem?

    parser = argparse.ArgumentParser()
    sub = parser.add_subparsers()
    sub.add_parser("info")
    parser.add_argument("paths", "+")
    parser.parse_args(["foo", "bar"])

    What is the expected output? What do you see instead?
    Expected behavior is that, failing to match one of the subparser inputs
    ("info"), the parser checks if the argument matches any of the top-level
    arguments, in this case the 'paths' multi-arg. In other words, it should be
    possible to make the subparser be optional, such that when the subparser
    argument fails to retrieve any valid subparser, the remaining args are
    parsed as if no subparser exists. At present, it does not seem possible to
    make a subparser be optional at all.
    Perhaps this could be exposed to the user as:

    parser.add_subparsers(nargs=argparse.OPTIONAL)

    or something to that effect. Or, allow a default subparser to be specified.
    I.e.,

    sub = parser.add_subparsers()
    info = sub.add_parser("info")
    main = sub.add_parser("main")
    sub.default = main

    I'm sure the point will come up that the current behavior is correct,
    because given a subparser like "info", a user could easily make a mistake
    like "myapp ino foo bar" and rather than get a safe error be given
    something unexpected. For this reason, I think the default behavior is
    usually going to be correct. BUT, it would still be nice if it could be
    optional, so that developers could be free to make that call. Sometimes the
    potential user errors aren't really an issue, and having to explicitly set
    a subparse arg every time can be a nuissance.

    @nvie nvie mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jul 13, 2010
    @nvie
    Copy link
    Mannequin Author

    nvie mannequin commented Jul 13, 2010

    Changed the title, so it shows that the feature request is for argparse.

    @nvie nvie mannequin changed the title optional subparsers argparse: optional subparsers Jul 13, 2010
    @bitdancer
    Copy link
    Member

    I've added Steven as nosy so he knows this was reposted here. I've also set the priority to low. Personally I'm at least -0 on this, since if I use a command that has subcommands I expect to get an error if I supply an invalid subcommand. As you say, however, the command designer *could* be supplied with the opportunity to shoot themselves in the foot :)

    The issue isn't going to go anywhere unless someone proposes a patch, though.

    @nvie
    Copy link
    Mannequin Author

    nvie mannequin commented Jul 14, 2010

    Actually, this is a rather common concept. Broadly used tools like for example Git use this kind of subcommand handling.

    This command shows all remotes:
    git remote (i.e. is like git remote list)

    Showing/removing remotes is done using subsubcommands:
    git remote show [...]
    git remote rm [...]

    That, in combination with the explicit design goal that "[argparse] isn't dogmatic about what your command line interface should look like" should be enough reason to be wanting this, in my humble opinion.

    @bitdancer
    Copy link
    Member

    See also 9540, which has an alternate proposal (that I don't like as much) for how to handle parser arguments supplied after subparsers are declared.

    Reviewing this, I'm now +1 on fixing this *somehow*, since clearly there is an ambiguity here that needs to be resolved.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Aug 10, 2010

    Seems like there's minimally the bug that argparse should currently throw an error if you add an argument after subparsers (since that argument will never be parsed under the current semantics).

    I do believe that supporting an optional command like the "git remote" example is useful, but as RDM suggests, this probably won't go anywhere unless someone proposes a patch.

    @elsdoerfer
    Copy link
    Mannequin

    elsdoerfer mannequin commented Aug 10, 2010

    To expand on my case from bpo-9540, I have a bunch of commands, each of which should enable a specific subset of options only available the individual command, but all of the commands share the same behavior in taking nargs='*' positional arguments:

    ./script.py --global-option command --command-option arg1 arg2 arg3

    For example:

    ./backups.py -c /etc/tarsnap.conf make --no-expire job1 job2

    If no positional arguments are given, all jobs defined in the config file are run. Or, in the above example, only "job1" and "job2" are run.

    The positional arguments are the same for *all* commands. Now I can define them separately for each subparser, which is what I'm currently doing, but I kind of like having the global usage instructions (script.py -h) indicating the fact that positional arguments can be passed after the command.

    In fact, right now I'm able to sort of achieve this by defining the positional nargs arguments both globally (to have them show in usage) and in each subparser (to have them parsed). This wouldn't be possible anymore if argparse where to throw an error after adding arguments after a subparser, although probably a more correct behavior.

    Anyway, while the two issues are clearly related, I don't think that the two are necessarily mutually exclusive. argparse could allow both optional subparsers (if no subparser matches), as well as pass control back to the parent parser once an already matched subparser is no longer able to handle further command line input. Or optionally, support defining subparsers as "options only", so that positional arguments would always be handled by the parent parser.

    Now, I can see how this could potentially become messy if we start talking about these positional arguments handled by the parent then being followed by more flags, which would then presumably also be handled by the parent etc. On the other hand, my use case doesn't seem that strange to me.

    @merwok
    Copy link
    Member

    merwok commented Aug 11, 2010

    Stable releases don’t go into stable branches, so I’m editing versions. I also remove 3.3 since it doesn’t exist now, it means “this won’t go in 3.2”.

    @merwok
    Copy link
    Member

    merwok commented Aug 11, 2010

    Wow, it is late. I wanted to write: New features don’t go into stable branches.

    @G2P
    Copy link
    Mannequin

    G2P mannequin commented Nov 15, 2010

    Trying to spec this, here is a proposed API:

        parser = argparse.ArgumentParser()
        sub = parser.add_subparsers(default='show')
        sub_show = sub.add_parser('show')
        sub_add = sub.add_parser('add')

    If default isn't passed, the subcommand isn't optional.
    If default is passed, and no explicit subcommand is given,
    the default subcommand is picked.
    Arguments are given to the top parser; passing arguments
    to the subcommand requires naming it explicitly.

    As far as motivation, I'd like to change a program that
    uses --choice options (that can have a default) to use
    more expressive subcommands. Some programs rely on implicit
    subcommands a lot; the ip command on linux is a good
    example.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Nov 16, 2010

    I think the proposed API looks fine and should be backwards compatible since add_subparsers will currently throw an exception with a default= argument.

    In case someone feels like writing a patch, you'll want to look at _SubParsersAction.__init__, which will need to grow the default= argument, and pass a different nargs= argument on. I think you'll need to define a new nargs type which means you probably also need to look at ArgumentParser._get_nargs_pattern as well.

    @bewest
    Copy link
    Mannequin

    bewest mannequin commented Sep 24, 2011

    I spent some time looking at this, as I was interested in
    using this pattern to simulate what git and hg do. I
    considered a few modifications and then found this bug. I
    think the default keyword passed to
    _SubParsersAction.__init__ makes sense.

    I started on a patch, that looks promising, but I'm having
    trouble getting the regexp right.

    Here's a changeset higlighting where I think the
    problematic regexp is:
    https://bitbucket.org/bewest/argparse/changeset/938e1e91ddd0

    https://gist.github.com/1202975#file_test_opt_subcommand.py
    Is the meager little test I put together.

    @bewest
    Copy link
    Mannequin

    bewest mannequin commented Sep 24, 2011

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Dec 15, 2011

    If you can make your patch relative to the cpython source tree, and add a couple tests, it will be easier to review.

    Thanks for working on this!

    @bewest
    Copy link
    Mannequin

    bewest mannequin commented Jan 6, 2012

    Ok, Steven, that sounds reasonable.

    I checked out git-svn python and started comparing diffs... I'm a little confused. What version of argparse should be patched to provide this feature?

    My HG version from https://code.google.com/p/argparse/ seems to contain a version of argparse 1.2 while, my git-svn checkout of python seems to contain an argparse 1.1. Should I attempt to bring cpython's version up to date as well, or attempt to strip out the version bump changes?

    @merwok
    Copy link
    Member

    merwok commented Jan 7, 2012

    You should work in the 3.3 standard library, i.e. on Lib/argparse.py in the default branch of the CPython Mercurial repository. See the devguide for more info. Thanks!

    @bewest
    Copy link
    Mannequin

    bewest mannequin commented Jan 7, 2012

    Thanks Eric. I was thrown by this document: http://wiki.python.org/moin/Git which describes fetching the sources from SVN using git. I'm comfortable doing either, but it doesn't resolve my confusion.

    The version of argparse in the python checkout is 1.1: http://hg.python.org/cpython/file/default/Lib/argparse.py
    64 __version__ = '1.1' but differs from the SVN version.

    whereas the argparse version available via google code is 1.2. The diffs indicate several changes not related to the change I'm attempting to make, which prevent my patch from applying cleanly. Looks like the HG version includes the 1.2 code... but I'm not sure why it would differ from SVN's trunk.

    @bewest
    Copy link
    Mannequin

    bewest mannequin commented Jan 7, 2012

    Ok, here's a rough attempt at stubbing this out against a python checkout. Will try to look at adding tests.

    (BTW, subsequent GETs should not modify the bug tracker... this seems like a bug since GET should be idempotent, but SFTN from the double posting.)

    @merwok
    Copy link
    Member

    merwok commented Jan 9, 2012

    Thanks for persevering in the face of VCS complications :) I have added a warning to the obsolete Git wiki page; I can’t do anything for the argparse Google code page. Anyway, trust us that argparse in the 3.3 stdlib is the place where development happens. (The Python Subversion repository is now dead.)

    As for the tracker, well, its use of HTTP and URIs is somewhat idiosyncratic. It’s far from perfect and definitely not as elegant or REST-compliant that one could wish for. Anyway, it’s just a tool that serves us rather well; I’ve never seen a double submission issues like here before.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Feb 20, 2012

    The implementation looks along the right track. Now it just needs some tests.

    @bewest
    Copy link
    Mannequin

    bewest mannequin commented Sep 14, 2012

    https://gist.github.com/1202975#file_test_opt_subcommand.py

    I sketched out a sloppy test earlier. I think this test is probably not quite comprehensive enough, and I'm not sure it fits into the python style either. I suppose there are other tests I can more or less copy.

    @wking
    Copy link
    Mannequin

    wking mannequin commented Jan 18, 2013

    Since 1 it seems like subparsers *are* optional by default. At least I get “error: too few arguments” for version 70740 of Lib/argparse.py, but no error for version 70741. It looks like this may be an unintentional side effect, since I see no mention of subparsers in bpo-10424.

    @zzzeek
    Copy link
    Mannequin

    zzzeek mannequin commented Feb 10, 2013

    um, this seems like a regression/bug? I now have users complaining that my apps are broken because of this change as of Python 3.3. My application is supposed to return the "help" screen when no command is given. Now I get a None error because argparse is not trapping this condition:

    from argparse import ArgumentParser
    parser = ArgumentParser(prog='test')
    subparsers = parser.add_subparsers()
    subparser = subparsers.add_parser("foo", help="run foo")
    parser.parse_args()
    $ python3.2 test.py
    usage: test [-h] {foo} ...
    test: error: too few arguments
    
    $ python3.3 test.py
    $

    This seems very much like a major feature has been yanked away from argparse, now I have to check for this condition explicitly.

    am I on the right issue here or do I need to open something new ?

    @seblu
    Copy link
    Mannequin

    seblu mannequin commented Apr 4, 2013

    I got the same issue that mike bayer with argparse doesn't throw error when subparser are missing.

    Is it a bug which should be fixed in Python or in all python script? This sounds like an API break.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Apr 9, 2013

    I think this problem arises from a change made in http://bugs.python.org/issue10424

    Changeset to default (i.e. development) is
    http://hg.python.org/cpython/rev/cab204a79e09

    Near the end of _parse_known_args it removes a:

        if positionals:
           self.error(_('too few arguments'))

    with a scan for required options that have not been seen.

    Ordinary positionals are required. But a SubParsersAction is not required. So we no longer get a warning.

    http://bugs.python.org/issue12776 changed this block of code as well. Notice the 2.7 and 3.2 branches have this 'too few arguments' error, but the default does not.

    The default value for Action.required is False. In _get_positional_kwargs(), a positional's required is set based on nargs (e.g. '+' is required, '*' not). But add_subparsers() does not use this, so its 'required' ends up False.

    This fudge seems to do the trick:

       parser = ArgumentParser(prog='test')
       subparsers = parser.add_subparsers()
       subparsers.required = True
       subparsers.dest = 'command'
       subparser = subparsers.add_parser("foo", help="run foo")
       parser.parse_args()

    producing an error message:

    usage: test [-h] {foo} ...
    test: error: the following arguments are required: command

    subparsers.dest is set so the error message can give this positional a name.

    I'll try to write a patch to do something similar in argparse itself.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Apr 10, 2013

    Further observations:

    parser.add_subparsers() accepts a 'dest' keyword arg, but not a 'required' one. Default of 'dest' is SUPPRESS, so the name does not appear in the Namespace. Changing it to something like 'command' will produce an entry, e.g. Namespace(command=foo, ...). Is this a problem?

    Assuming we have a clean way of assigning a name to 'subparsers', what should it be? 'command', '{cmd}', '{foo,bar,baz}' (like in the usage line)? This name also could be used when telling the user the subparser choice is invalid (parser._check_value).

    This issue exposes a problem with '_get_action_name()'. This function gets a name from the action's option_strings, metavar or dest. If it can't get a string, it returns None.

    ArgumentError pays attention to whether this action name is a string or None, and adjusts its message accordingly. But the new replacement for the 'too few arguments' error message does a ', '.join([action names]), which chokes if one of those names is None. There is a mutually_exclusive_groups test that also uses this 'join'. This bug should be fixed regardless of what is done with subparsers error messages.

    So the issues are:

    • making 'subparsers' a required argument
    • choosing or generating an appropriate name for 'subparsers'
    • passing this name to the error message (via _get_action_name?)
    • correcting the handling of action names when they are unknown (None).

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Apr 13, 2013

    This patch addresses both issues raised here:

    • throw an error when the subparser argument is missing
    • allow the subparser argument to be optional

    argparse.py:

    _SubParsersAction -
    add 'required=True' keyword.

    name(self) method - creates a name of the form {cmd1,cmd2} for error messages.

    _get_action_name() - try action.name() if it can't get a name from option_strings, dest or metavar. Still can return None.

    2 error cases do a join on a list of action_names. If a name is None, this will choke. Add a ['%s'%x for x in list] guard.

    test_argparse.py:
    add cases to the subparser block to test the 'required' keyword, and to test the error message changes.

    argparse.rst:
    add a summary of the add_subparsers() arguments.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 30, 2013

    "msg113512 - (view) Author: Steven Bethard (bethard)
    Seems like there's minimally the bug that argparse should currently throw an error if you add an argument after subparsers (since that argument will never be parsed under the current semantics)."

    This isn't quite right. If the main usage signature is:

    usage: PROG [-h] foo {one,two} ... baz

    the parser._match_arguments_partial() method will allocate the 1st string to 'foo', the last to 'baz', and pass the rest to the subparser(s). It doesn't know how many the subparsers can use, but it knows that 'baz' requires one. From the standpoint of matching argument strings and arguments, a subparser is essentially a '+' positional.

    On the other hand if 'baz' (the positional after the subparser) was '*' or '?' it would not get any strings.

    If it is possible that subparser(s) doesn't need all the strings passed to it, the user could use 'parse_known_args', and deal with the unparsed strings themselves (possibly with another parser).

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Apr 10, 2014

    http://stackoverflow.com/questions/22990977/why-does-this-argparse-code-behave-differently-between-python-2-and-3

    is answered by this change in how required arguments are tested, and how subparsers fell through the cracks.

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Apr 29, 2014

    Another Stackoverflow question triggered by this issue

    http://stackoverflow.com/questions/23349349/argparse-with-required-subparser

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jun 7, 2016

    My answer to

    http://stackoverflow.com/questions/23349349/argparse-with-required-subparser

    is getting a slow but steady stream of + scores; so the required subparser issue is still bothering people.

    This particular question addresses the problem that the error message has when a required subparser is missing - it can't format the error with the default dest - SUPPRESS. The may be a another bug issue that addresses that.

    Anyways, due to this continued attention, I'm going to raise the priority for this issue.

    @asottile
    Copy link
    Mannequin

    asottile mannequin commented Aug 15, 2017

    I've attempted to address some of the backward/forward compatibility issue with subparsers becoming optional by default (vs required by default in python2) with this pull request: #3027 (would love to get a review as well!)

    @merwok
    Copy link
    Member

    merwok commented Sep 15, 2017

    I am now reviewing the PR added to the other issue by Anthony. This ticket has a lot of discussion; it would be good to check which parts are addressed by the other ticket, and particularly if the problems noted by Mike and others are now fixed.

    @asottile
    Copy link
    Mannequin

    asottile mannequin commented Sep 15, 2017

    My patch mainly addresses the regression pointed out by mike bayer (zzzeek)'s comment.

    @merwok merwok added the 3.7 (EOL) end of life label Sep 20, 2017
    @merwok
    Copy link
    Member

    merwok commented Sep 20, 2017

    The other PR is now merged in 3.7, and won’t be backported (it changes default behaviour and adds a new param).

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Oct 18, 2017

    In a recent stackoverflow question a user wanted this optional-subparsers ability in Python 2.7.

    https://stackoverflow.com/questions/46667843/how-to-set-a-default-subparser-using-argparse-module-with-python-2-7

    Short of modifying the _parse_known_args method, the best I could suggest was a two stage parsing. That is, one parser without the subparsers. This uses parse_known_args, and if a 'cmd' is provided passes the 'extras' to one that handles subparsers.

    ---

    Another issue which I don't think has been addressed is the 'usage' when subparsers are optional. At least with 3.5, subparsers are displayed with the choices: {'cmd1', 'cmd2', ...}, but no indication of being optional. An optional positional (with ? nargs) would normally be displayed as

     prog [-h] [{'one', 'two'}] ...
    

    My guess is that 'usage' adds the [] when positionals nargs='?', without regard to the 'required' attribute (I should verify this from code).

    I'm undecided as to whether we want the brackets or not. It's more accurate, but makes the usage messier. And the 'help' grouping for 'optional-positionals' is the subject of other bug/issue(s).

    I haven't checked it the patch has changed this behavior.

    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-feature A feature request or enhancement
    Projects
    Status: Features
    Development

    No branches or pull requests

    2 participants