python / cpython

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

Using parameter 'action' in function 'argparse.ArgumentParser.add_subparsers' #92811

Open MatthiasHeinz opened 2 years ago

MatthiasHeinz commented 2 years ago

Bug report The function _argparse.ArgumentParser.addsubparsers throws an error regarding _parserclass, when passing the parameter action.

Sample-code "ArgParserTest.py":

from argparse import ArgumentParser

parser = ArgumentParser(prog='Testcase for ArgumentParser._subparsers')
# init subparsers for parser
subparsers = parser.add_subparsers(action='count') # Any action works to reproduce.
# Add my_subparser with any "action"
my_subparser = subparsers.add_parser(name='my_subparser')

Terminal output:

> python ArgParserTest.py
Traceback (most recent call last):
  File "REDACTED\ArgParserTest.py", line 5, in <module>
    subparsers = parser.add_subparsers(dest='subcommands', action='count', help='Available subcommands. Run \'<subcommand> -h\' to display further information.')
  File "REDACTED\Python3.10.4\lib\argparse.py", line 1798, in add_subparsers
    action = parsers_class(option_strings=[], **kwargs)
TypeError: _CountAction.__init__() got an unexpected keyword argument 'parser_class'

Possible resolutions

  1. If there is no valid action to subparsers: a. Have the error-message complain about the argument action instead of _parserclass. b. Remove the argument action from the parameter list to the _addsubparser-function. c. Update the documentation accordingly.
  2. If action can be a valid argument, fix either the error-message or whatever error did occur.

Environment

hpaulj commented 2 years ago

An 'action' works as long as it's compatible with the default subparsers class. For example using that subclass itself:

 subparsers = parser.add_subparsers(action=argparse._SubParsersAction)

add_subparsers is essentially a variant on add_argument, one that creates a positional and returns an Action. It is special in that it creates a special Action subclass, and supports any necessary parameters.

While I've suggested using 'action' to supply a customized _SUbParsersAction class, I haven't seen other users or questioners try to use it (either on bug/issues or StackOverflow). While refining the docs might be order, it might just add to the confusion.

In other words, 'action' works if the subclass signature is like:

class _SubParsersAction(Action):   
     def __init__(self,
             option_strings,
             prog,
             parser_class,
             dest=SUPPRESS,
             required=False,
             help=None,
             metavar=None):
MatthiasHeinz commented 2 years ago

I was trying to use subparsers for the first time and basically tried to use action='extend' to be able to supply multiple of those subcommands at the same time (i.e. have a help for both command1 and command2 and offer the ability to either run command1, command2 or both of them in a single call to the program - which apparently does not work this way).

Anyway, it should at least be possible to refine the error-message TypeError: _CountAction.__init__() got an unexpected keyword argument 'parser_class', right? Because that one is really questionable from a user's point of view, when supplying an action argument.

While I do see your point regarding the possible confusion in the docs, I doubt anyone will be able to use this feature/parameter at all, if it stays undocumented. Unfortunately reading the docs and even staring at the sourcecode for a couple of minutes did not yield me any useful information in this situation - just my two cents.

hpaulj commented 2 years ago

You are trying to get around an intentional limitation. If you try to use add_subparsers twice, argparse raises a "'cannot have multiple subparser arguments" error. When a sub-parser is done, the main parser does not resume parsing. It just cleans up a bit and exits.

The 'action' parameter is designed to be flexible and extensible. Registered words like "extend" map onto Action subclasses (e.g. _ExtendAction). add_argument does not tightly control what parameters it accepts and passes on. Users can define their own subclasses, usually modelled on one of the existing subclasses.

add_subparsers creates the Action with a call:

 action = parsers_class(option_strings=[], **kwargs)

add_argument does something similar

action = action_class(**kwargs)

The add_argument signature is about as general as it gets, add_argument(self, *args, **kwargs). It only does a modest amount of checking before it passes kwargs on. It's the class __init__ that determines what keyword parameters are allowed. To refine the error message that you got, the action __init__ would have to accept a general **kwargs, and test that for keys that it does not recognize or want. None of the subclasses do that.

Use of action='append' and 'extend' make most sense with flagged, optionals. While not prohibited for positionals, they don't make much sense there.

MatthiasHeinz commented 2 years ago

Sorry, if my previous post was misleading is any way. In the first paragraph, I was just trying to tell you how people, that are new(ish) to argparse, could end up trying to invoke the add_subparsers function with the parameter action and a value which is covered by the documentation.

And my goal certainly wasn't for you to rewrite the entire code because of some required argument. I was just thinking about adding a sanity check for the parameter action before calling into

# create the parsers action and add it to the positionals list
parsers_class = self._pop_action_class(kwargs, 'parsers')
action = parsers_class(option_strings=[], **kwargs)
self._subparsers._add_action(action)

of the add_subparsers function. Maybe something along the lines of

# Alternatively blacklist actions not having the aforementioned subclass signature
if supplied_action_parameter is not None and isinstance(supplied_action_parameter, str):
        raise CustomException('<Some more/precise details than the error message about parser_class...>')

Link to aforementioned subclass signature

This way, the default action strings (like count, append, _storetrue, ...), which aren't supposed to work in this context (if I understand you correctly), would trigger an error message, that's tailored to this situation.

ruzito commented 2 years ago

I do not understand why this is intentional limitation. If I want to create nested subparsers I would potentially want to know the whole chain of commands taken. So for example:

sub = parent.add_subparsers(dest='subparsers', action='append')
a = sub.add_parser('a')
subsub = a.add_subparsers(dest='subparsers', action='append')
b = subsub.add_parser('b')

would return

Namespace(subparsers=['a', 'b'])

I do not see a simpler way of doing this using the current ideology of argparse library right now.

hpaulj commented 2 years ago

An 'append' action cannot work across subparsers. It is best to give main and subparsers arguments different dest. Otherwise you'll only see the subparser values or defaults.

backerdrive commented 9 months ago

@hpaulj I am missing the same feature @ruzito mentioned.

Given is a command-line requirement like:

(cmd1|cmd2) subcmd

This can be expressed by

parser = argparse.ArgumentParser()
subparsers = parser.add_subparsers(dest="command")
subparser_a = subparsers.add_parser("cmd1")
subparser_b = subparsers.add_parser("cmd2")
subsubparsers_a = subparser_a.add_subparsers(dest="command")
subsubparser_a = subsubparsers_a.add_parser("subcmd")
subsubparsers_b = subparser_b.add_subparsers(dest="command")
subsubparser_b = subsubparsers_b.add_parser("subcmd")
print(parser.parse_args(["cmd1", "subcmd"]))

We wouldn't be able to identify, if subsubparser_a or subsubparser_b actually parsed the input, assuming the need to process output further conditionally. The result is ambigious:

Namespace(command='subcmd')

I'd like to assign an identifier or unique tuple (whole chain of commands) to dest, resulting in:

Namespace(command=["cmd1", "subcmd"])

Giving different dest argument names is not scalable for multiple chained parsers and not as easy to process:

Namespace(command1='cmd1',command2='subcmd', ... ,commandXZY='subcmdXYZ')

This is what lists/sequences are naturally used for.

Adding action='append' would be just one of several alternatives. What about being able to additionally pass a callback to dest, which receives dest values of parent parsers?

dest=lambda prev_cmds: ".".join(prev_cmds)

I hope this is comprehensible so far. As this gets a bit orthogonal to OP, should I make up a separate feature request?

hpaulj commented 9 months ago

This ability to nest subparsers is a feature, simply the result of how the subparser mechanism is coded, not an intentional design. So I think it's unreasonable to expect special enhancements.

Normally action is a string, which is registered with an Action subclass. Or it's a user specified subclass. In either case it defines the class of the argument created by add_argument, or in this case add_subparsers. action doesn't just specify how the dest is assigned; it defines an Action subclass.

When you read the code for add_subparsers, you'll see that it fetches the class corresponding to action='parsers', which is registered as _SubParsersAction. It's that subclass that provides all the subparser action - adding parsers (aschoices`), and passing the remaining argv strings to the chosen subparser.

 self.register('action', 'parsers', _SubParsersAction)
 ...
  # create the parsers action and add it to the positionals list
    parsers_class = self._pop_action_class(kwargs, 'parsers')
    action = parsers_class(option_strings=[], **kwargs)
    self._subparsers._add_action(action)

The __call__ for the subparsers Action sets the command dest, if any with:

    def __call__(self, parser, namespace, values, option_string=None):
          parser_name = values[0]
         arg_strings = values[1:]
        # set the parser name if requested
        if self.dest is not SUPPRESS:
            setattr(namespace, self.dest, parser_name)

That parser_name is used to select which subparser will be used:

         parser = self._name_parser_map[parser_name]

This is what allows us to use aliases for the commands. You might be able to play games with those. Or a custom subclass could set a special dest based on the parser_name and its 'own name'.

Anyways, I don't think the standard argparse should have any added features to allow/enhance this use of nested subparsers. This nesting is a kludge used to get around the intended one-subparser per parser constraint. But if you want to write your own add_subparsers and/or _SubParsersAction class, feel free.

Message ID: @.***>

backerdrive commented 9 months ago

@hpaulj Thanks for pointing out the invoked code locations, it makes sense to me.

Anyways, I don't think the standard argparse should have any added features to allow/enhance this use of nested subparsers. This nesting is a kludge used to get around the intended one-subparser per parser constraint.

This was not clear to me from reading the docs. In this case it probably would be a good idea to document intended usage - only one direct subparser layer, no nesting - more clearly.

If the goal is to identify the (possibly nested) used sub-parser afterwards, I have found a simple workaround for dest via set_defaults:

# Above example, with below additions
subsubparser_a.set_defaults(__ID__=["cmd1", "subcmd"])
subsubparser_b.set_defaults(__ID__=["cmd2", "subcmd"])

, which provides __ID__ in returned Namespace:

$ ./path/to/prog.py cmd1 subcmd
Namespace(__ID__=['cmd1', 'subcmd'])
kiner-shah commented 1 month ago

Any updates on this issue? I am facing a similar issue when trying to append subparsers. It throws:

TypeError: _AppendAction.__init__() got an unexpected keyword argument 'parser_class'
hpaulj commented 1 month ago

add_subparsers is a specialized version of add_argument. It intrinsically specifies an action subclass, a subparser handler. append is another action subclass. action doesn't just add properties, so one can't modify the other.

Also add_subparsers explicitly prohibits repeats. Once the main parser passes parsing to the subparser, it is done. No going back for more main parsing. You trying to find a sneaky way around this structure.

On Wed, Sep 18, 2024, 1:29 AM Kiner Shah @.***> wrote:

Any updates on this issue? I am facing a similar issue when trying to append subparsers. It throws:

TypeError: _AppendAction.init() got an unexpected keyword argument 'parser_class'

— Reply to this email directly, view it on GitHub https://github.com/python/cpython/issues/92811#issuecomment-2357832346, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAITB6CO2LOPPQZBYEVHC43ZXE2XNAVCNFSM5V6EHBYKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMZVG44DGMRTGQ3A . You are receiving this because you were mentioned.Message ID: @.***>

hpaulj commented 1 month ago

You seem to misunderstand what action='append' does. An append action (subclass) differs from the default store only in how it writes new values to the args Namespace. store simply writes the value, overwriting anything that was there before (for that particular dest). That could be a default value (written at the start of parsing) or a previous value. append appends the new value to a list.

append only makes sense with an optional, since the flag string (e.g. --foo) can be repeated. A positional can not be repeated, since there's no flag string; it is 'triggered' by position only. Technically you could define another positional with the same dest, and use append, but in that case, why not use a multiple nargs.

A subparsers Action is a positional, with a + like nargs - the first string is the command (subparser name), and the remaining strings are passed as arguments to the subparser. If there are strings it can't handle, they are put in an extras list, and returned via parse_known_args or trigger an error.

Allowing only one subparser is not an incidental constraint that can be removed by simply changing a few lines of code or allowing some new action argument. It's an inherent part of how the feature operates.

On Wed, Sep 18, 2024 at 1:29 AM Kiner Shah @.***> wrote:

Any updates on this issue? I am facing a similar issue when trying to append subparsers. It throws:

TypeError: _AppendAction.init() got an unexpected keyword argument 'parser_class'

— Reply to this email directly, view it on GitHub https://github.com/python/cpython/issues/92811#issuecomment-2357832346, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAITB6CO2LOPPQZBYEVHC43ZXE2XNAVCNFSM5V6EHBYKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMZVG44DGMRTGQ3A . You are receiving this because you were mentioned.Message ID: @.***>

serhiy-storchaka commented 3 weeks ago

I see two requests here:

  1. Make add_subparsers() supporting actions 'count', 'append', etc.

This will not happen. This would require half of argparser to be rewritten, and this is unlikely to be possible with backward compatibility.

  1. Produce more user friendly error message when the user specifies wrong action. In particularly if the string is specified.

This looks simply, but you can register _SubParsersAction-compatible action and pass a string as action to add_subparsers(). This is a supported feature. A subclass check of the result will not help either -- it should not be a _SubParsersAction subclass, it can be a callable compatible with _SubParsersAction.

So there is a risk to break somebody's sophisticated code. On other hand, better error message will help users who make this error first time. Second time they will not make such error or will be able to quickly recognize it.

hpaulj commented 3 weeks ago

Kiner,

add_subparsers is like add_argument in that it creates an Action subclass instance, using a kwargs dict of parameters. add_subparsers adds a parser_class value to that kwargs. The registered 'parsers' action, _SubParsersAction expects such a parameter (see its __init__). But all the other defined subclasses, including the one that's registered as 'append', do not expect such a parameter. That's why you got that error - 'parser_class' does not make any sense to a class that just appends simple values to a Namespace dest list.

So while it is possible to specify a custom action class for handling subparsers, it has to be compatible with the default one. To do that you need to understand the relevant parts of the argparse.py file. Action subclasses are not designed for any sort of additive or cumulative behavior.

Paul

On Wed, Sep 25, 2024 at 12:35 PM Serhiy Storchaka @.***> wrote:

I see two requests here:

Make add_subparsers() supporting actions 'count', 'append', etc.

This will not happen. This would require half of argparser to be rewritten, and this is unlikely to be possible with backward compatibility.

Produce more user friendly error message when the user specifies wrong action. In particularly if the string is specified.

This looks simply, but you can register _SubParsersAction-compatible action and pass a string as action to add_subparsers(). This is a supported feature. A subclass check of the result will not help either -- it should not be a _SubParsersAction subclass, it can be a callable compatible with _SubParsersAction.

So there is a risk to break somebody's sophisticated code. On other hand, better error message will help users who make this error first time. Second time they will not make such error or will be able to quickly recognize it.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>