brentyi / tyro

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

Mutable default arguments are double initialized #149

Open ElleryXii opened 1 month ago

ElleryXii commented 1 month ago

A minimum replicable example:


from dataclasses import dataclass, field
import tyro
import random

@dataclass
class TestDataclass:
    val: int = 1
    def __post_init__(self):
        self.val *= 2

def func1(data:TestDataclass=TestDataclass()):
    print(data.val)

if __name__ == "__main__":
    func1()  # prints 2
    tyro.cli(func1) # prints 4

When using tyro.cli, the __post_init__ function is called twice on the same data instance. This also happens to non-data class instances' __init__ functions.

tyro version: 0.8.5

brentyi commented 1 month ago

Hi, I appreciate the succinct example!

Unfortunately this seems difficult to make less confusing. If you run the CLI, these flags would give you:

# print `2`
python example.py --data.val 1
# print `4`
python example.py --data.val 2

In your func1 signature, you currently pass in a default TestDataclass instance. tyro inspects the values in this default instance to populate default values for the CLI flags; it sees that val is set to 2 so it assigns the default value of --data.val to 2. Open to suggestions, but I'm not sure how we would work around this without removing the ability to override argument-level default values entirely.

The easiest fix would probably be to eliminate the __post_init__ mutation:

@dataclass
class TestDataclass:
    val: int = 1

    @property
    def doubled_val(self):
        return self.val * 2
ElleryXii commented 1 month ago

Hi, thank you for the reply, I can see why it's tricky. It's not a big issue to sidestep, but having an appropriate warning/note in the documentation might be helpful.

brentyi commented 1 month ago

but having an appropriate warning/note in the documentation might be helpful.

Makes sense! I'll add it to my to-do list; if you have suggestions on where or how to put this in the docs I'd also be interested.