SimpleParsing icon indicating copy to clipboard operation
SimpleParsing copied to clipboard

Subparsers "steal" parsing from the parent

Open otaj opened this issue 5 years ago • 2 comments

Describe the bug Hi, thanks for the amazing work!

I am honestly not sure, whether this should be classified as a bug or not, because it is a default behaviour of argparse, however, there are workarounds.

What happens is that whenever parser encounters a sub command, it moves all the parsing to this subparser and does not return back, meaning, that any options from the parent parser are invalid after any subparser command. Which as I said, is a "standard" behaviour of argparse, but I am convinced it is wrong and it would be clearer if it was circumvented.

To Reproduce Steps to reproduce the behavior:

  1. Use example from the examples/subparsers and save as tmp.py
from dataclasses import dataclass
from typing import *
from pathlib import Path
from simple_parsing import ArgumentParser, subparsers

@dataclass
class Train:
    """Example of a command to start a Training run."""
    # the training directory
    train_dir: Path = Path("~/train")

    def execute(self):
        print(f"Training in directory {self.train_dir}")


@dataclass
class Test:
    """Example of a command to start a Test run."""
    # the testing directory
    test_dir: Path = Path("~/train")

    def execute(self):
        print(f"Testing in directory {self.test_dir}")


@dataclass
class Program:
    """Some top-level command"""
    command: Union[Train, Test]
    verbose: bool = False   # log additional messages in the console.
    
    def execute(self):
        print(f"Executing Program (verbose: {self.verbose})")
        return self.command.execute()


parser = ArgumentParser()
parser.add_arguments(Program, dest="prog")
args = parser.parse_args()
prog: Program = args.prog

print("prog:", prog)
prog.execute()
  1. Try to run it as python tmp.py train --verbose
usage: tmp.py [-h] [--verbose bool] {train,test} ...
tmp.py: error: unrecognized arguments: --verbose

Expected behavior I expect to be able to parse optional arguments from parent dataclass even after specifying subcommand, i.e. python tmp.py train --verbose == python tmp.py --verbose train, since verbose is optional.

Additional context It is possible to circumventing it by creating "temporary" parser for all the parent options and give this temporary parser as parent_parsers argument to the created subparsers, however, it is not possible to cleanly do with simple-parsing, it would have to be implemented on simple-parsing level

otaj avatar Dec 17 '20 13:12 otaj

Hey there! Great first issue!

Indeed, this is one of the annoying things with subparsers in argparse. One idea I played around with was to add an attempt_to_reorder argument to parser.parse_known_args, which tries to re-order the arguments automatically to avoid this issue.

One problem with this, however, is that by re-ordering the arguments as python tmp.py --verbose train, the parser would try to parse a boolean value out of train, and wouldn't see any value for the command argument. Therefore, you can solve this in a few ways:

  • Explicitly pass a value for each argument, as in python tmp.py train --verbose=True (which will become python tmp.py --verbose=True train and work fine)
  • Or, Use flag(False) from the simple_parsing.helpers.fields module, such that the nargs is changed from "1" to "*"
  • (Or, doesn't currently work): Have Optional[bool] as the type of for verbose, and use it as usual without passing a value.

As an example, here is what you'd want, which we can get to work by just slightly modifying the example, following the second option described above:

# (...) same as above


from simple_parsing.helpers.fields import flag

@dataclass
class Program:
    """Some top-level command"""
    command: Union[Train, Test]
    verbose: bool = flag(False)   # log additional messages in the console.
    
    def execute(self):
        print(f"Program (verbose: {self.verbose})")
        return self.command.execute()


parser = ArgumentParser()
parser.add_arguments(Program, dest="prog")
args, unused_args = parser.parse_known_args(attempt_to_reorder=True)
prog: Program = args.prog

print("prog:", prog)
prog.execute()
$ python examples/subparsers/subparsers_example.py train --verbose
Unparsed arguments when using subparsers. Will attempt to automatically re-order the unparsed arguments ['--verbose'].
prog: Program(command=Train(train_dir=PosixPath('~/train')), verbose=True)
Program (verbose: True)
Training in directory ~/train
$ python examples/subparsers/subparsers_example.py --verbose train
prog: Program(command=Train(train_dir=PosixPath('~/train')), verbose=True)
Program (verbose: True)
Training in directory ~/train

Let me know what you think, and thanks again for posting! :)

lebrice avatar Dec 27 '20 21:12 lebrice

Nice, attempt_to_reorder seems like a very nice option. Didn't try it yet on anything more substantial, but just wondering, how much of reordering is going on there? Will it work with nested subparsers?

Anyway, I believe, that since simple-parsing is subclassing ArgumentParser, it can do this kind of thing automatically :)

And, as I suggested in the previous post, one of the options I know to be working (but not easily implementable on top of simple-parsing is to create a dummy parser for all arguments except the subparser ones and then give this to parser as argument to parent_parsers. I might try to create a PR for it, but I have to get more familiar with the code in order to do that :)

Thanks for this awesome module!

otaj avatar Dec 28 '20 20:12 otaj