lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
428 stars 52 forks source link

Subparsers "steal" parsing from the parent #40

Open otaj opened 3 years ago

otaj commented 3 years ago

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
  2. 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

lebrice commented 3 years ago

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:

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! :)

otaj commented 3 years ago

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!