brentyi / tyro

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

tyro.conf.arg aliases not working in Pydantic>=2 #110

Closed mhuerta-endava closed 8 months ago

mhuerta-endava commented 8 months ago
# tyrotest.py
from typing import Annotated

import tyro
from pydantic import BaseModel
from tyro.conf import arg

class AliasCfg(BaseModel):
    alias: Annotated[str, arg(aliases=["-a"])]

cfg = tyro.cli(AliasCfg)
print(f"Alias: {cfg.alias}")

With Pydantic < 2:

$ python tyrotest.py
usage: tyrotest.py [-h] --alias STR

╭─ arguments ─────────────────────────────────────────────╮
│ -h, --help              show this help message and exit │
│ --alias STR, -a STR     (required)                      │
╰─────────────────────────────────────────────────────────╯

With Pydantic >=2:

$ python tyrotest.py
usage: tyrotest.py [-h] --alias STR

╭─ arguments ────────────────────────────────────────╮
│ -h, --help         show this help message and exit │
│ --alias STR        (required)                      │
╰────────────────────────────────────────────────────╯

and of course the alias is not recognized on invocation.

With functions and dataclasses the alias works perfectly.

brentyi commented 8 months ago

Hi Marcelo, thanks for filing this + the detailed reproduction instruction!

This is actually fixed in the branch for #106, which I'll try to merge + release today. I can add your example to the tests to prevent regressions.

brentyi commented 8 months ago

Should be fixed in 0.6.1!

mhuerta-endava commented 8 months ago

Should be fixed in 0.6.1!

Can confirm. Thanks!

scottstanie commented 7 months ago

Is there some reason we shouldn't be able to alter the annotations after the initial class definition?

# tyrotest.py
from typing import Annotated

import tyro
from pydantic import BaseModel
from tyro.conf import arg

class AliasCfg(BaseModel):
    alias: str

AliasCfg.__annotations__["alias"] = Annotated[str, arg(aliases=["-a"])]
cfg = tyro.cli(AliasCfg)
print(f"Alias: {cfg.alias}")

This does not print the -a, although it does show up if you use @dataclass.

(For background- I'm interested in adding aliases to nested configuration objects, but was first testing on a top-level annotation)

brentyi commented 7 months ago

Mechanically, the reason is that we use pydantic's model_fields dictionary to extract types + metadata. This dictionary will be generated before your annotations update here (I believe during BaseModel's __init_subclass__).

I have mixed feelings about this pattern , but here's an alternative that should work in pydantic v2:

from typing import Annotated

import tyro
from pydantic import BaseModel
from tyro.conf import arg

class AliasCfg(BaseModel):
    alias: str

AliasCfg.model_fields["alias"].metadata.append(arg(aliases=["-a"]))

cfg = tyro.cli(AliasCfg)
print(f"Alias: {cfg.alias}")
scottstanie commented 7 months ago

thank you! I'll try that out. I agree that it feels a little fragile to the inner workings of Pydantic... but I'm not sure of a better way to do this- the configuration class is defined elsewhere, and doesn't know anything about tyro.conf.arg. I'm open to ideas about that if you have any.

brentyi commented 7 months ago

Ok!

I guess another option is to subclass and override just the fields you want to add aliases for:

# tyrotest.py
from typing import Annotated

import tyro
from pydantic import BaseModel
from tyro.conf import arg

class ExternalCfg(BaseModel):
    alias: str

class AliasedCfg(ExternalCfg):
    alias: Annotated[str, arg(aliases=["-a"])]

cfg = tyro.cli(AliasedCfg)
print(f"Alias: {cfg.alias}")

Although maybe this doesn't work if it's deeply nested.