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

Option name disappears with OmitArgPrefixes #185

Open
KrobotP opened this issue Oct 23, 2024 · 4 comments
Open

Option name disappears with OmitArgPrefixes #185

KrobotP opened this issue Oct 23, 2024 · 4 comments

Comments

@KrobotP
Copy link

KrobotP commented Oct 23, 2024

Let me just demonstrate the issue on example. Having following data structures:

class MyRange(NamedTuple):

    low: int
    high: int

    def __str__(self):
        return f"<{self.low}, {self.high}>"

    @staticmethod
    def tyro_constructor(range_str: Annotated[
        str,
        tyro.conf.arg(name=""),
    ],):
        import re

        m = re.match("([0-9]+)(-([0-9]+))*", range_str)
        low = m[1]
        high = low if not m[3] else m[3]

        return MyRange(low, high)

    @staticmethod
    def tyro_constructor_set(range_str_set: Annotated[
            Set[str],
            tyro.conf.arg(name=""),
        ],):
        return {MyRange.tyro_constructor(r) for r in range_str_set}


class MySpec(BaseModel):
    some_set: Set[int] = Field(
        default={1, 2, 3},
        decsription="Some set of integers",
        title="Some set",
    )

    some_string: str = Field(
        decsription="Some string without a default value.",
        title="SomeSTR",
    )

    here_comes_the_trouble: Annotated[
        MyRange,
        tyro.conf.arg(constructor=MyRange.tyro_constructor_set),
    ] = Field(
        default={MyRange(0, 1024)},
        decsription="I would like this one in the same group as others",
        title="Please help",
    )

Now I build an application:

def add_spec(spec: MySpec):
    print(spec.here_comes_the_trouble)

if __name__ == "__main__":
    tyro.cli(add_spec)

and everything works as expected:

$ python3.11 example.py --help
usage: example.py [-h] [--spec.some-set [INT [INT ...]]] --spec.some-string STR [--spec.here-comes-the-trouble [STR [STR ...]]]

╭─ options ───────────────────────────────────────────────╮
│ -h, --help              show this help message and exit │
╰─────────────────────────────────────────────────────────╯
╭─ spec options ──────────────────────────────────────────╮
│ --spec.some-set [INT [INT ...]]                         │
│                         (default: '{1, 2, 3}')          │
│ --spec.some-string STR  (required)                      │
│ --spec.here-comes-the-trouble [STR [STR ...]]           │
│                         (optional)                      │
╰─────────────────────────────────────────────────────────╯

Now, I would like to remove spec. prefixes, so I use tyro.conf.OmitArgPrefixes:

def add_spec(spec: Annotated[MySpec, tyro.conf.OmitArgPrefixes]):
    print(spec.here_comes_the_trouble)

if __name__ == "__main__":
    tyro.cli(add_spec)

Now, here-comes-the-trouble has been consumed together with its prefix:

$ python3.11 example.py --help
usage: example.py [-h] [--some-set [INT [INT ...]]] [--some-string ANNOTATED|ANNOTATED] [-- [STR [STR ...]]]

╭─ options ───────────────────────────────────────────────╮
│ -h, --help              show this help message and exit │
│ --some-set [INT [INT ...]]                              │
│                         (default: '{1, 2, 3}')          │
│ --spec.some-string STR  (required)                      │
│ -- [STR [STR ...]]      (optional)                      │
╰─────────────────────────────────────────────────────────╯
@brentyi
Copy link
Owner

brentyi commented Oct 23, 2024

Hi @KrobotP, thanks for filing this issue! This seems like expected behavior with the current API, since --here-comes-the-trouble is actually a prefix and not the argument name itself.

I have a design revision in mind for the "custom constructor" API which should be much more flexible and cover this use case. I think I can have it done within a week but time is tight so it's hard to make promises.

A workaround right now is to skip the custom constructor API, add here_comes_the_trouble annotated with Set[str], and a method or property that will convert it to Set[MyRange].

@KrobotP
Copy link
Author

KrobotP commented Oct 24, 2024

Hi @brentyi, thanks for the clarification and a workaround. However, changing types is not possible as other stuff depends on it. But its OK for me to stick with prefixes for a while if there is a hope for better future :).

@KrobotP
Copy link
Author

KrobotP commented Oct 24, 2024

One more idea to tyro.conf.OmitArgPrefixes: It might provide finer control if this option would affect only the "next prefix level". Using this approach:

def add_spec(spec: Annotated[MySpec, tyro.conf.OmitArgPrefixes]):
    print(spec.here_comes_the_trouble)

will remove only spec. prefixes, while no others are affected. If I would like to remove prefixes in nested options in MySpec, I would have to use tyro.conf.OmitArgPrefixes on particular options.

@brentyi
Copy link
Owner

brentyi commented Oct 24, 2024

Yeah I see! This will work for "erasing" spec from the prefix list:

def add_spec(spec: Annotated[MySpec, tyro.conf.arg(name="")]):
    print(spec.here_comes_the_trouble)

I've learned a lot from working with the current iteration of the tyro.conf API; will avoid breaking changes but these things should get much more user-friendly once I get around to revisiting it.

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

No branches or pull requests

2 participants