brentyi / tyro

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

Pydantic Default Fields for Nested Models are marked as required #108

Closed tungalbert99 closed 10 months ago

tungalbert99 commented 10 months ago

I'm trying to provide defaults of nested pydantic models but the defaults don't show up and are requested as "required".

Steps for reproduction:

from typing import Literal

import pydantic
import tyro

SourceFlaskSize = Literal[125, 250, 500]

class FlaskConfig(pydantic.BaseModel):
    id: str
    size: SourceFlaskSize
    initial_volume: int
    desired_end_volume: int | None = None

class Config(pydantic.BaseModel):
    source_flasks: list[FlaskConfig] = pydantic.Field(
        [FlaskConfig(id="Flask 1", size=250, initial_volume=50)],
        min_length=1,
        max_length=8,
    )

    source_flask: FlaskConfig = pydantic.Field(
        FlaskConfig(id="Flask 1", size=250, initial_volume=50)
    )

    test_list: list[int] = pydantic.Field([5], min_length=1, max_length=8)

if __name__ == "__main__":
    test_item = tyro.cli(Config, prog="hello")

It's hard to tell if the min_length/max_length requirements work with tyro but in any case the error is that the default FlaskConfig is provided in pydantic but tyro seems to think it's not.

image

tungalbert99 commented 10 months ago

Wondering if this is a pydantic specific thing 🤔 since it seems to work in an example https://brentyi.github.io/tyro/examples/02_nesting/04_nesting_in_containers/

brentyi commented 10 months ago

Yes sorry for the trouble here, I think this is the same issue as #107!

tungalbert99 commented 10 months ago

One thing I'm still noticing is that the list of nested models is a little wonky. I'm not able to add a second FlaskConfig in the specification?

brentyi commented 10 months ago

Sorry yeah, that seems like expected behavior -- unfortunately we don't currently support appending to dynamically-sized lists of structures like this.

If you have a field like flasks: list[FlaskConfig] = [FlaskConfig(...), FlaskConfig(...), FlaskConfig(...)], tyro will currently let you override values in each FlaskConfig object, but we don't have the ability to remove elements from the list or append new ones.

I imagine this is what you're seeing, where you have a length-1 default list and all tyro lets you do is override values in this list. A similar behavior also shows up in the color_tuple_alt: tuple[Color, ...] field in this nesting example.

Adding more functionality here is probably doable with a nontrivial amount of thought, in the meantime some workarounds I think are unideal but could work are:

class Config(pydantic.BaseModel): source_flasks: list[FlaskConfig]

tungalbert99 commented 10 months ago

I like the tuple idea thank you!