Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] tapify with subparsers #140

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kddubey
Copy link
Contributor

@kddubey kddubey commented Jun 28, 2024

Fix #101

Not ready for a full review yet, but I'd appreciate feedback on the overall approach

I'll do all of the # TODO: s and add tests soon

I'll also study fire.Fire's interface and behavior a bit more to understand what ppl want out of this type of feature

@kddubey kddubey marked this pull request as draft June 28, 2024 06:26
@kddubey
Copy link
Contributor Author

kddubey commented Jun 28, 2024

Demo
# demo_subparsers.py
from typing import List, Optional

from tap import tapify_with_subparsers


class Class:
    def __init__(self, arg_int: int, arg_bool: bool = True, arg_list: Optional[List[int]] = None):
        """
        :param arg_int: some integer
        :param arg_list: some list of integers
        """
        self.arg_int = arg_int
        self.arg_bool = arg_bool
        self.arg_list = arg_list

    def my_method(self, x: int) -> int:
        """Summary of my method

        :param x: an integer
        :return: another integer
        """
        return self.arg_int + x

    def my_other_method(self) -> Optional[List[int]]:
        """Summary of my other method

        :return: hmmm
        """
        return self.arg_list if self.arg_bool else None

    def __repr__(self) -> str:
        return f"{self.arg_int=} {self.arg_bool=} {self.arg_list=}"


if __name__ == "__main__":
    print(tapify_with_subparsers(Class))

Running—

python demo_subparsers.py -h

—outputs:

usage: demo_subparsers.py --arg_int ARG_INT [--arg_bool] [--arg_list [ARG_LIST ...]] [-h] {my_method,my_other_method} ...

positional arguments:
  {my_method,my_other_method}
                        sub-command help
    my_method           my_method help
    my_other_method     my_other_method help

options:
  --arg_int ARG_INT     (int, required) some integer
  --arg_bool            (bool, default=True)
  --arg_list [ARG_LIST ...]
                        (Optional[List[int]], default=None) some list of integers
  -h, --help            show this help message and exit

Running—

python demo_subparsers.py my_method -h

—outputs:

usage: demo_subparsers.py my_method --x X [-h]

my_method description

options:
  --x X       (int, required)
  -h, --help  show this help message and exit

Running—

python demo_subparsers.py --arg_int 2 my_method --x 110

—outputs:

112

Running—

python demo_subparsers.py --arg_list 1 2 3 --arg_int 2 my_other_method

—outputs:

[1, 2, 3]

@kddubey
Copy link
Contributor Author

kddubey commented Jun 28, 2024

The name-collision issue is weird. Play w/ this argparse script:

# demo_names.py
import argparse

parser = argparse.ArgumentParser()
parser.add_argument("--verbose", default=0, type=int)

subparsers = parser.add_subparsers(dest="parser_name")

parser_lan = subparsers.add_parser("car")
parser_lan.add_argument("--boo")
parser_lan.add_argument("--verbose")  # collides w/ name in the main parser

parser_serial = subparsers.add_parser("bus")
parser_serial.add_argument("--fun")

print(parser.parse_args())

For example, running—

python demo_names.py --verbose 1 car --boo booval

outputs:

Namespace(verbose=None, parser_name='car', boo='booval')

Seems unexpected.

Not sure how to handle it: be consistent w/ argparse behavior, or attempt to fix it by separating the namespaces for the main parser and subparsers as in, e.g., here. Or (what I think I'll go with for now) raise an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[suggestion] tapify(...) should support classes/objects by producing subargument parsers
1 participant