brentyi / tyro

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

Tyro 0.9.1 issue with _primitive_spec #203

Open ljstella opened 3 hours ago

ljstella commented 3 hours ago

Stack Trace:

Traceback (most recent call last):
  File "/home/runner/work/contentctl/contentctl/contentctl/contentctl.py", line 193, in main
    config = tyro.cli(models)
             ^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_cli.py", line 169, in cli
    output = _cli_impl(
             ^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_cli.py", line 373, in _cli_impl
    parser_spec = _parsers.ParserSpecification.from_callable_or_type(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_parsers.py", line 123, in from_callable_or_type
    field_out = handle_field(
                ^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_parsers.py", line 340, in handle_field
    subparsers_attempt = SubparsersSpecification.from_field(
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_parsers.py", line 550, in from_field
    subparser = ParserSpecification.from_callable_or_type(
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_parsers.py", line [13](https://github.com/splunk/contentctl/actions/runs/11894663466/job/33142404486?pr=302#step:6:14)3, in from_callable_or_type
    if field_out.lowered.required:
       ^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.10/x64/lib/python3.11/functools.py", line 1001, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_arguments.py", line [19](https://github.com/splunk/contentctl/actions/runs/11894663466/job/33142404486?pr=302#step:6:20)1, in lowered
    _rule_apply_primitive_specs(self, lowered)
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/_arguments.py", line 327, in _rule_apply_primitive_specs
    lowered.default = spec.str_from_instance(arg.field.default)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/.cache/pypoetry/virtualenvs/contentctl-Nb2OUnIL-py3.11/lib/python3.11/site-packages/tyro/constructors/_primitive_spec.py", line [25](https://github.com/splunk/contentctl/actions/runs/11894663466/job/33142404486?pr=302#step:6:26)5, in <lambda>
    else instance.name
         ^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'name'
There was a serious issue where the config file could not be created.
The entire stack trace is provided below (please include it if filing a bug report).

Getting this exact same stack trace on python3.11, 3.12, and 3.13 with the newest release with everything we try to do with our CLI. This codebase worked fine with 3.11 and 3.12 w/ tyro==0.8.14. Looks like there's some new changes, specifically tyro/constructors/_primitive_spec.py that are causing an issue for us, specifically here:

str_from_instance=lambda instance: [
                (
                    str(instance.value)
                    if _markers.EnumChoicesFromValues in type_info.markers
                    else instance.name
                )
            ],

Is there an issue here, or are there docs we can go through to port things so they continue to work going forward if this behavior is expected?

brentyi commented 2 hours ago

Hi @ljstella, thanks for filing the issue!

I can reproduce the error message you're seeing with this example:

import enum

import tyro

class SomeEnum(enum.StrEnum):
    A = enum.auto()
    B = enum.auto()

def main(x: SomeEnum = "A"):  # this should create a type error!
    print(x)

tyro.cli(main)

In contentctl, it looks like a similar case is hit because you have use_enum_values=True in your Pydantic models, which unfortunately creates many situations where the static type (an enum) doesn't match the runtime type (a string).

It's hard for me to generally support type mismatches like this, but I can add a patch + tests today to suppress this specific error.

ljstella commented 2 hours ago

Hi @brentyi

Thanks for the super quick response! We're looking to migrate away from using use_enum_values anyway- I thought that might be the issue, but wasn't positive. I would greatly appreciate a patch fix but if you can't or don't want to maintain it, we can move some things around and remove the use_enum_values config that was causing this issue.