brentyi / tyro

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

Tyro marks attrs fields with default values as "fixed" #125

Closed shermansiu closed 7 months ago

shermansiu commented 7 months ago

Unlike for dataclasses, tyro marks fields with default values as being "fixed to" the default value.

The tyro version is 0.7.2 and the attrs version is 23.2.0, on Python 3.10.

shermansiu commented 7 months ago

It does not matter if the dataclass/attrs class is frozen or not, the bug persists in either case.

brentyi commented 7 months ago

Thanks for reporting this @shermansiu!

Are you able to share a runnable example?

That sounds unideal. I spotted some attrs-related issues when I was trying to reproduce this (fixed in #126), but couldn't figure out the specific behavior you're describing.

This works as expected for me:

from __future__ import annotations

from attrs import define, field

import tyro

@define
class Args:
    x: int = field(default=2)
    y: tuple[int, ...] = field(default=(1, 2, 3))
    z: int = field(default=5)

if __name__ == "__main__":
    args = tyro.cli(Args)
    print(args)
shermansiu commented 7 months ago

Thanks, I think #126 fixes this issue.

With the sample script you provided, I get the following error for the dev version before #126:

$ python test.py --help
usage: test.py [-h] [--x {fixed}] [--y {fixed}] [--z {fixed}]

Method generated by attrs for class Args.

╭─ options ──────────────────────────────────────────╮
│ -h, --help         show this help message and exit │
│ --x {fixed}        (fixed to: 2)                   │
│ --y {fixed}        (fixed to: 1 2 3)               │
│ --z {fixed}        (fixed to: 5)                   │
╰────────────────────────────────────────────────────╯
$ python test.py
Args(x=2, y=(1, 2, 3), z=5)
$ python test.py --x 3
╭─ Value error ────────────────────────────────────────────────────────────────╮
│ Error parsing --x: --x was passed in, but is a fixed argument that cannot be │
│ parsed                                                                       │
│ ──────────────────────────────────────────────────────────────────────────── │
│ Argument helptext:                                                           │
│     --x {fixed}                                                              │
│     (fixed to: 2)                                                            │
│ ──────────────────────────────────────────────────────────────────────────── │
│ For full helptext, see test.py --help

After the commit, everything works as expected.

brentyi commented 7 months ago

Oops, for posterity I meant to write:

from attrs import define, field

import tyro

@define
class Args:
    x: int = field(default=2)
    y: tuple[int, ...] = field(default=(1, 2, 3))
    z: int = field(default=5)

if __name__ == "__main__":
    args = tyro.cli(Args)
    print(args)

which was working without the __future__ import.

Glad things are fixed for you!

shermansiu commented 6 months ago

Thanks! Looking forward to the next release, when I can use the version from PyPi and not a custom installation!

brentyi commented 6 months ago

Should be available already 🙂