swansonk14 / typed-argument-parser

Typed argument parser for Python
MIT License
494 stars 40 forks source link

Add useful type hints to `Tap.add_subparser` #111

Closed aucampia closed 9 months ago

aucampia commented 1 year ago

The type signature is def add_subparser(self, flag: str, subparser_type: type, **kwargs) -> None: which more or less eliminates most of the point of having type hints, it would be much better if it had useful type hints instead of **kwargs.

aucampia commented 1 year ago

This problem appears to be present all over, really, if you are open to improving this I would be happy to help.

nierob commented 1 year ago

One could argue that this example:

class SubparserA(Tap):
    bar: int  # bar help

class SubparserB(Tap):
    baz: str

class Args(Tap):
    foo: bool = False  # foo help

    def configure(self):
        self.add_subparsers(help="sub-command help")
        self.add_subparser("a", SubparserA, help="a help")
        self.add_subparser("b", SubparserB, help="b help")

Could be transformed to something like:


class SubparserA(Tap):
    """a help"""
    bar: int  # bar help

class SubparserB(Tap):
    """b help"""
    baz: str

class Args(Tap):
    foo: bool = False  # foo help

    a: SubparserA |None
    b: SubparserB |None

That way one could build the whole argument tree declarative

aucampia commented 1 year ago

Another place where this is a problem is Tap.add_argument which has:

def add_argument(self, *name_or_flags, **kwargs) -> None:

Which again obscures most of the type hints for add_arguments from the superclass (i.e. ArgumentParser.add_arguments).

aucampia commented 1 year ago

Also to be clear the problem with this approach is that type hints and docstrings are masked, which really makes it painful to work with Tap. Again I'm happy to fix this if you are open to this, I will copy all the type hints and docstrings from upstream (i.e. argparse).

Never mind, there are no doc strings, so the only real problem are the type-hint, arg-name erasure.

martinjm97 commented 9 months ago

@aucampia,

Thank you for raising this concern! As you realized, the issue with kwargs is that they are not documented upstream in argparse. With regard to not having the type of the subparser, we believe that it cannot be known statically. For instance, in

class SubparserA(Tap):
    bar: int  # bar help

class SubparserB(Tap):
    baz: str

class Args(Tap):
    foo: bool = False  # foo help

    def configure(self):
        self.add_subparsers(help="sub-command help")
        self.add_subparser("a", SubparserA, help="a help")
        self.add_subparser("b", SubparserB, help="b help")

either "a" or "b" could be selected. As a result, either of the two subparsers could be used at runtime.

@nierob, Thank you for the suggestion. We think your recommendation is a good extension to the current syntax that makes it easier to use subparsers.

We will break that into a separate issue and close this issue for now.

Thanks, JK