brentyi / tyro

CLI interfaces & config objects, from types
https://brentyi.github.io/tyro
MIT License
467 stars 23 forks source link

List of str works, but list of str constructors fails #148

Open rggjan opened 2 months ago

rggjan commented 2 months ago

Running the below code with --help fails with the error:

tyro._instantiators.UnsupportedTypeAnnotationError: For variable-length sequences over nested types, we need a default value to infer length from.

Commenting the line list_of_str_constructors works fine.

This means the constructor function works fine (converting a string to a Foo), and also parsing a list of strings works fine. However, parsing a list of Foo object fails, even though the constructor function should make it possible to pass a regular list of strings to the command line, which I’d expect the constructor function to then convert to Foo objects.

import dataclasses
from typing import List

import attr
from typing_extensions import Annotated
import tyro

@dataclasses.dataclass(frozen=True)
class Foo:
    name: str
    last_name: int

    @staticmethod
    def from_string(full_name: str) -> 'Foo':
        name, last_name = full_name.rsplit(' ', 1)
        return Foo(name=name, last_name=last_name)

@attr.s(auto_attribs=True)
class Bar:

    str_var: str
    str_constructor: Annotated[Foo, tyro.conf.arg(constructor=Foo.from_string)]

    list_of_str: List[str]
    list_of_str_constructors: List[Annotated[Foo, tyro.conf.arg(constructor=Foo.from_string)]]

x = tyro.cli(Bar)
print(x)
brentyi commented 2 months ago

Hi @rggjan, thanks for filing this issue!

To check my understanding, it sounds like the behavior you're expecting is for:

Unfortunately this is indeed not supported right now, although I can see why you'd expect it to be. If you'd like a workaround the easiest is to make sure tyro.conf.arg() is applied to the outermost type:

    list_of_str_constructors: Annotated[
        List[Foo], tyro.conf.arg(constructor=Foo.from_string_list)
    ]

which requires a new constructor:

    @staticmethod
    def from_string_list(full_names: List[str]) -> "List[Foo]":
        return [Foo.from_string(name) for name in full_names]

For the discrepancy you're seeing, the longer explanation is that we handle annotations like x: list[str] and x: list[int] differently from how we handle annotations like x: list[SomeDataclass]. The former case is converted to a single argument for the whole list (--x 1 2 3), while in the latter case we convert each item in the list into its own group of arguments (--x.0.field to set a field for the 0th item in the list). For custom constructors the code path we're taking is closer to the latter case, which is causing the problem you're seeing. I should improve the error message at least; I do also hope to lift this requirement, but that would be a much more time-consuming change.

rggjan commented 1 month ago

Thanks a lot, that’s exactly what I meant, and your proposed workaround seems to work! However, with the attached new code, I know get this nested argument --list-of-str-constructors.full-names, even with prefix_name=False. Is this expected or a bug? Is there some way to be able to call it with just --full-names or --list-of-str-constructors?

import dataclasses
from typing import List

import attr
from typing_extensions import Annotated
import tyro

@dataclasses.dataclass(frozen=True)
class Foo:
    name: str
    last_name: int

    @staticmethod
    def from_string_list(full_names: List[str]) -> "List[Foo]":
        return [Foo.from_string(name) for name in full_names]

    @staticmethod
    def from_string(full_name: str) -> 'Foo':
        name, last_name = full_name.rsplit(',', 1)
        return Foo(name=name, last_name=last_name)

@attr.s(auto_attribs=True)
class Bar:
    list_of_str_constructors: Annotated[
        List[Foo], tyro.conf.arg(constructor=Foo.from_string_list, prefix_name=False)
    ]
x = tyro.cli(Bar)
print(x)
brentyi commented 1 month ago

For --full-names, you can use tyro.conf.OmitArgPrefixes:

    list_of_str_constructors: Annotated[
        List[Foo], tyro.conf.arg(constructor=Foo.from_string_list), tyro.conf.OmitArgPrefixes
    ]

or tyro.conf.arg(prefix_name=False) for the full_names argument itself:

    @staticmethod
    def from_string_list(
        full_names: Annotated[List[str], tyro.conf.arg(prefix_name=False)],
    ) -> "List[Foo]":

For --list-of-str-constructors, you can "erase" the name of full_names:

    @staticmethod
    def from_string_list(
        full_names: Annotated[List[str], tyro.conf.arg(name="")],
    ) -> "List[Foo]":

The reason for the problem that you're seeing is that prefix_name=False in tyro.conf.arg() was meant to configure only a single argument, so it doesn't get applied recursively. No breaking changes expected, but I do plan to revisit the design + docs for all of this.

rggjan commented 1 month ago

Perfect, thanks so much for the help!