typed-argument-parser icon indicating copy to clipboard operation
typed-argument-parser copied to clipboard

Support for argument groups

Open rggjan opened this issue 5 years ago • 14 comments

Is support for argument groups considered / planned?

rggjan avatar Jan 14 '20 12:01 rggjan

Definitely worth considering. For the sake of keeping a record, this is the link to the argparse description: https://docs.python.org/2/library/argparse.html#argument-groups

martinjm97 avatar Jan 14 '20 16:01 martinjm97

In the meantime, what would be an elegant way to still achieve some sort of argument grouping / subparser functionality without sacrificing cool features like code completion? The only thing can think of is defining arguments in different subclasses that inherit from subclass(es) of Tap. Or is there a better way?

Thanks!

Jane333 avatar Nov 04 '20 08:11 Jane333

Hi @Jane333,

Thank you for the question! In our most recent release, we added subparsers! The main challenge for subparsers was to find an elegant and typing-friendly way to pass in collections of arguments. In the case of subparsers, the subparser is a Tap so it can be passed in as an argument as shown in the example below:

class SubparserA(Tap):
    bar: int

class Args(Tap):
    foo: bool = False

    def configure(self):
        self.add_subparser('a', SubparserA, help='a help')

Perhaps an analogous solution will work for adding argument groups as well? We're still thinking about the best way to add argument grouping, but we'll hopefully have a solution soon. I'll think about what the best stop-gap solution is and get back to you.

martinjm97 avatar Nov 04 '20 15:11 martinjm97

Thank you for your quick answer!

I'm already using the newest version (1.6.0), however, with the code you posted, code completion only works for variables declared in Args, not those declared in Subparser:

class SubparserA(Tap):
    bar: int

class Args(Tap):
    foo: bool = False

    def configure(self):
        self.add_subparser('a', SubparserA, help='a help')

args = Args().parse_args('a --bar 10'.split())

print(args.foo)  # code completion works
print(args.bar)  # code completion does not work

I think I'll stick to solving this using inheritance. Thank you for a really cool library!:-)

Jane333 avatar Nov 04 '20 17:11 Jane333

Explicitly setting args to the union type resolves this issue, although it's a little extra work :)

args: Union[Args, SubparserA] = Args().parse_args('a --bar 10'.split())

print(args.foo)  # code completion works
print(args.bar)  # code completion works

Perhaps this indicates a flaw in our documentation as it stands! Thank you for pointing this out.

martinjm97 avatar Nov 04 '20 19:11 martinjm97

That works, thanks!

Unfortunately, Unions open up a new world of problems for me with mypy type checking. Example:


def test_func(var: Union[Args, SubparserA], boolean: bool):
    if boolean:
        print(var.foo)
    else:
        print(var.bar)

argument = Args()
test_func(argument, boolean=True)

error: Item "Args" of "Union[Args, SubparserA]" has no attribute "bar"

There seems to be no real solution, just workarounds:

https://stackoverflow.com/questions/51782177/mypy-attribute-error-in-union-with-textio-type

Anyway, I'm already quite happy with the inheritance solution, just posting this here for anyone else who might need it.

Thanks again!

Jane333 avatar Nov 06 '20 20:11 Jane333

@martinjm97 I am noticing that the type returned in the above example is always Args but with self values populated according to SubparserA rather than ever returning SubparserA type itself. I noticed this because I am trying to add a function to SubparserA (and not Args), but am receiving method not defined because the returned object is of type Args.

Additionally, its difficult to delegate that function from Args because it seems to not contain any mention of the underlying SubparserA anywhere thats easy to get to.

Any thoughts?

kyprifog avatar Mar 29 '21 17:03 kyprifog

Hi @kyprifog,

What an interesting thought! For example,

class SubparserA(Tap):
    bar: int

class Args(Tap):
    foo: bool = False

    def configure(self):
        self.add_subparser('a', SubparserA, help='a help')

# args is of type Args
args = Args().parse_args('a --bar 10'.split())  

Is it a problem that args will be of type Args? If it's of type Args, we don't have access to the methods defined for SubparserA. If it's of type SubparserA, then we don't have access to Args's member variables and methods. Furthermore, the type of args isn't really Union[Args, SubparserA] either because it lacks the methods of SubparserA. As-is, the system matches the functionality of argparseand I'm not sure if this is a clean solution. Ideas are welcome.

Thanks again for the interesting thought!

Best, Jesse and Kyle

martinjm97 avatar Apr 14 '21 20:04 martinjm97

Maybe this would clarify a bit. I'm was using typed argument parsers in the following way, this includes the way that I was getting around the issue that I outlined above:


class MasonConfig(Tap):
    
    def configure(self):
        self.add_subparsers(help='sub-command help')
        self.add_subparser('merge', MergeConfig, help='Merge config')
        
    def run_job(self):
        if self.job_type == "merge":
            return MergeJob(self.input_path, self.output_path, self.input_format, self.parse_headers).run_job()
        

class MergeConfig(Tap):
    job_type: str = "merge"
    input_path: str
    output_path: str
    input_format: Literal['csv']
    parse_headers: bool = False

class MergeJob(Runnable):
    input_path: str
    output_path: str
    format: str
    parse_headers: bool = False

    def run_job(self):
        print("Running Merge Job")

class Runnable:

    @abstractmethod
    def run_job(self) -> str:
        raise NotImplementedError("Run job not implemented")

>> config = MasonConfig().parse_args()
>> config.run_job()
Running Merge Job

There is a couple of ways around this but it seems like if there is a level of comfort with adding new attributes to the underlying config object then the way subparser should work is to return a nested object for the subparser. For example:

class MergeConfig(Tap, Runnable):
    input_path: str
    output_path: str
    input_format: Literal['csv']
    parse_headers: bool = False

    def run_job(self):
        print("Running Merge Job")

>>  config: MasonConfig = MasonConfig().parse_args()
>>  sub_config: MergeConfig = config.merge
>>  sub_config.run_job()
Running Merge Job

or even better:

class OtherConfig(Tap, Runnable):
    attribute: str

    def run_job(self):
        print("Running Other Job")


>>  config: MasonConfig = MasonConfig().parse_args()
>>  sub_config: Union[MergeConfig, OtherConfig] = config.job
>>  sub_config.run_job()

kyprifog avatar Apr 15 '21 13:04 kyprifog

Hi @kyprifog,

We see what you're thinking and we think it accords with some of the discussion that people have been having around saving files with nested arguments (https://github.com/swansonk14/typed-argument-parser/issues/40#issuecomment-820230448). While we think this is a great idea, our understanding is that treating subparsers in this way conflicts with the behavior of argparse and would break existing code. Thank you for the detailed and thoughtful comments. Let us know if we're mistaken.

Best, Kyle and Jesse

martinjm97 avatar Jun 20 '21 15:06 martinjm97

I'm personally using subparsers as a way to separate arguments handling depending on the command specified. I then use the main parser only as a "command parser". For that specific use-case, here's a way to do it.

class ConfigArgs(Tap):
    some_arg: str

class AnalyzerArgs(Tap):
    another_arg: int

class CommandParser(Tap):
    def configure(self):
        self.add_subparsers(dest='command') # This is where the subparser used will be stored
        self.add_subparser('config', ConfigArgs, help='View or change the configuration')
        self.add_subparser('analyze', AnalyzerArgs, help='Run the analyzer')

args = CommandParser().parse_args()

if args.command == 'config':
    run_config(args)
elif args.command == 'analyze':
    run_analyzer(args)

def run_config(args: ConfigArgs):
    pass

def run_analyzer(args: AnalyzerArgs):
    pass

Of course, we are humans and we could still manage to mismatch the commands and the actions, but that's exactly why tests exists ;)

Reference : argparse documentation for add_subparsers()

vigenere23 avatar Oct 28 '21 13:10 vigenere23

Hi all,

We’re working on sketching a plan for a Tap version 2 release. We have a sketch of the solution to a number of issues including this one. This includes a breaking change to the treatment of subparsers and a different way that we treat comments to support argument groups. We’re excited to start the implementation and would love feedback and recommendations from everyone. We’re so appreciative of all of the wonderful pull requests, issues, and ideas from all of you!

Best, Kyle and Jesse

swansonk14 avatar Feb 06 '22 02:02 swansonk14

Hi, I just stumbled on the doc and added some comments!

Stavr0sStuff avatar Mar 11 '22 09:03 Stavr0sStuff

Another thing argument groups allow is mutually exclusive arguments, which show nicely in the help as being as such.

LiraNuna avatar Sep 24 '22 16:09 LiraNuna