-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allowing a 'default subcommand value' for a custom constructor which returns a Union #221
Comments
(please see last edit at the end of this message) def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[AdamConfig, tyro.conf.subcommand(name="adam")],
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd")],
]
return cfgs # type: ignore
CLIOptimizer = tyro.conf.AvoidSubcommands[Union[*_constructor()]]
@dataclass
class Config:
optimizer: CLIOptimizer
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) For
I can now make it aware some default is being passed if I add a default for the dataclass field ( def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[AdamConfig, tyro.conf.subcommand(name="adam")],
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd")],
]
return cfgs # type: ignore
CLIOptimizer = tyro.conf.AvoidSubcommands[Union[*_constructor()]]
@dataclass
class Config:
optimizer: CLIOptimizer = SGDConfig()
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) Running
Notice that the default that was passed in the dataclass field and the one in the CLI are different - def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd")], # order changed!
Annotated[AdamConfig, tyro.conf.subcommand(name="adam")],
]
return cfgs # type: ignore
CLIOptimizer = tyro.conf.AvoidSubcommands[Union[*_constructor()]]
@dataclass
class Config:
optimizer: CLIOptimizer = AdamConfig()
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) then for
(notice again edit: from dataclasses import dataclass
from typing import Annotated, Union
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd")],
Annotated[AdamConfig, tyro.conf.subcommand(name="adam")],
]
return Union[*cfgs] # type: ignore
@dataclass
class Config:
optimizer: Annotated[
OptimizerConfig, tyro.conf.arg(constructor_factory=_constructor)
] = AdamConfig()
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
# config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) Once
Note that running this will not error out (and the AdamConfg() is assigned). So the question is how to make it pick up the correct default being passed, and importantly I would like to have both:
and the options for the default config being shown whenever
preferably with cfg = tyro.cli(
Config,
# config=(tyro.conf.ConsolidateSubcommandArgs,),
default=Config(optimizer=AdamConfig()),
) will have the same effect just described. later edit: from dataclasses import dataclass
from typing import Annotated, Union
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd", default=SGDConfig())],
Annotated[AdamConfig, tyro.conf.subcommand(name="adam", default=AdamConfig())],
]
return Union[*cfgs] # type: ignore
@dataclass
class Config:
optimizer: Annotated[
OptimizerConfig, tyro.conf.arg(constructor_factory=_constructor)
] = AdamConfig()
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
# config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) we get:
the default is now correctly assigned and if I run
|
Hi, thanks for the fix and sorry for the very long messages! After staring at the docs and playing around with the debugger for a long time I've realized it's possible this is intended behavior, so this might be a feature request (?). I guess I'm looking for a weaker version of [1] Or more precisely, the effect of the Marker would be as if the the argv was modified in the way described. |
Hi @mirceamironenco! Yeah, this is working as-designed but I do agree that what you've described would be ideal. We currently set all subcommands to required when Lines 600 to 602 in 1704284
It's also in the tests: Lines 833 to 835 in 1704284
And hinted at in the docs ("at the cost of support for optional subcommands"), although I can see that this is vague: tyro/src/tyro/conf/_markers.py Lines 65 to 67 in 1704284
I'm looking into relaxing this, which seems simple but is actually a little bit tricky (this was part of the reason for the current design). If we visualize the possible subcommands as a tree:
I can get back (hopefully today) once I've made some progress! |
On this: unfortunately not! It's hardcoded here: Lines 241 to 249 in 1704284
|
Thank you for adding this! I think the tradeoff with required arguments makes complete sense (and it's hard to even think of a use-case where this would be a blocker for anything). It seems right now it is possible to run from dataclasses import dataclass
from typing import Annotated, Union
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd")],
Annotated[AdamConfig, tyro.conf.subcommand(name="adam")],
]
return Union[*cfgs] # type: ignore
CLIOptimizer = Annotated[
OptimizerConfig,
tyro.conf.arg(constructor_factory=_constructor),
]
@dataclass
class Config:
optimizer: CLIOptimizer = AdamConfig(adam_foo=0.5)
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(Config, config=(tyro.conf.ConsolidateSubcommandArgs,))
main(cfg) The only question is, is there any way to add an option which allows the user to 'mark the defaults as having been chosen explicitly'? i.e. so that I would have no issue if this is some explicit opt-in/additional flag that goes with ConsolidateSubcommandArgs (or maybe in As a small side-note, for the following: from dataclasses import dataclass
from typing import Annotated, Union
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd", default=SGDConfig())],
Annotated[AdamConfig, tyro.conf.subcommand(name="adam", default=AdamConfig())],
]
return Union[*cfgs] # type: ignore
CLIOptimizer = Annotated[
OptimizerConfig,
tyro.conf.arg(constructor_factory=_constructor),
]
@dataclass
class Config:
optimizer: CLIOptimizer = AdamConfig(adam_foo=0.5)
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(Config, config=(tyro.conf.ConsolidateSubcommandArgs,))
main(cfg) running |
Unfortunately I can't think of a clean way to implement this without rewriting the The second discrepancy you pointed out: this behavior makes sense, but not as much sense as Thanks for helping me sort out all of these kinks...! |
That makes sense, it seems the limitation is on the side of argparse. Thanks for taking a look at this! One last question:
I found an alternate behavior that would be just as good for me, but I can only seem to trigger it if I turn off subcommands, however I'm not sure why that would be the case. For the following: from dataclasses import dataclass
from typing import Annotated, Union
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd", default=SGDConfig())],
Annotated[
AdamConfig,
tyro.conf.subcommand(name="adam", default=AdamConfig()),
],
]
return Union[*cfgs] # type: ignore
CLIOptimizer = Annotated[
OptimizerConfig,
tyro.conf.arg(constructor_factory=_constructor, help="Optimizer Hello"), # doc via help!
]
@dataclass
class Config:
optimizer: tyro.conf.AvoidSubcommands[CLIOptimizer] = AdamConfig() # avoid
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) for
however if I remove the AvoidSubcommands marker (and change nothing else): @dataclass
class Config:
optimizer: CLIOptimizer = AdamConfig()
foo: int = 1
bar: str = "abc" it goes away:
even though the same help text is still specified: CLIOptimizer = Annotated[
OptimizerConfig,
tyro.conf.arg(constructor_factory=_constructor, help="Optimizer Hello"),
] Is there any way to keep it? |
Sorry for the opaqueness here, this unfortunately also isn't configurable. To explain the current behavior: When you have optimizer: AdamConfig = AdamConfig()
"""Optimizer Hello""" , where the help string unambiguously applies to just the arguments of When you don't have optimizer: AdamConfig | SgdConfig = AdamConfig()
"""Optimizer Hello""" , where the help string applies to the union / subparser group as a whole. In this case, the helptext will show up above the subcommand list (in I don't feel strongly about this design. The tradeoffs are relatively minor:
|
I understand, thanks! I guess my usecase is complicated enough that I'll have to change |
I haven't decided whether to merge, but could you see if #227 does what you're looking for? The changes are small but there might also be bugs, the parser + subparser tree structures get complicated and (as you may have noticed) there are often edge cases I don't think of. |
In terms of behavior this is what I want (I assume you mean the group doc) but your concern relating False positives I think is valid. With the current implementation I can also see someone asking for the help text to update depending on the element of the Union that was chosen. I'm still playing around a bit, but I've noticed in this scenario 3 spots where a helptext/doc can be provided so maybe there is a way to keep the behavior the same when a subcommand choice hasn't been made, i.e.
and if a choice has been made I see 2 options which can be used either for the command description or the group subtitle (where 'Optimizer Hello' is now): from dataclasses import dataclass
from typing import Annotated, Union
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
"""Adam dataclass DOC""" # doc dataclass
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def _constructor() -> type[OptimizerConfig]:
cfgs = [
Annotated[SGDConfig, tyro.conf.subcommand(name="sgd", default=SGDConfig())],
Annotated[
AdamConfig,
tyro.conf.subcommand(
name="adam",
default=AdamConfig(),
description="Adam subcommand description", # doc subcommand
),
],
]
return Union[*cfgs] # type: ignore
CLIOptimizer = Annotated[
OptimizerConfig,
tyro.conf.arg(constructor_factory=_constructor, help="Optimizer hello"), # doc union
]
@dataclass
class Config:
optimizer: CLIOptimizer = AdamConfig()
foo: int = 1
bar: str = "abc"
def main(cfg: Config):
print(cfg)
if __name__ == "__main__":
cfg = tyro.cli(
Config,
config=(tyro.conf.ConsolidateSubcommandArgs,),
)
main(cfg) for
for
notice that """Adam dataclass DOC""" is not used anywhere, so I was thinking either that or "Adam subcommand description" could be propagated, making all 3 choices always opt-in for the user. The "Adam dataclass DOC" does show up if I remove the
Again, sorry if this not possible with the underlying implementation. I would understand not including this change. I'll try to also learn a bit more about tyro internals since I understand there is also a maintenance burden here with every proposal. edit: Just an idea, I'm also realizing a lot of the behavior I'm looking for occurs in the scenario where "multiple subcommands are possible (think CLIScheduler, CLIOptimizer, etc) + tyro.conf.ConsolidateSubcommandArgs specified + all subcommands have defaults / are provided". Maybe there is some way to pick up on this? So it would be the setting where in a larger config example, the user is specifying |
By the way, is this a bug? from dataclasses import dataclass
from typing import Annotated, Literal
import tyro
@dataclass(frozen=True)
class OptimizerConfig:
lr: float = 1e-1
@dataclass(frozen=True)
class AdamConfig(OptimizerConfig):
"""Adam dataclass DOC"""
adam_foo: float = 1.0
@dataclass(frozen=True)
class SGDConfig(OptimizerConfig):
sgd_foo: float = 1.0
def name_to_cfg(name: Literal["adam", "sgd"]) -> OptimizerConfig:
if name == "adam":
return AdamConfig()
if name == "sgd":
return SGDConfig()
CLIConfig = Annotated[OptimizerConfig, tyro.conf.arg(constructor=name_to_cfg)]
@dataclass
class Config:
optimizer: CLIConfig = AdamConfig()
if __name__ == "__main__":
print(tyro.cli(Config))
edit: actually it seems to be the same for 0.9.5 (?). Is there any way to turn it off in this case? Modifying |
This I didn't fully follow, do you mean to have on subcommand per possible combination of the Union options? For the "Default:" behavior, the code is here: Lines 158 to 171 in 0c2cb26
The reason is just to document what happens if def name_to_cfg(name: Literal["adam", "sgd"]) -> OptimizerConfig:
if name == "adam":
return AdamConfig()
if name == "sgd":
return SGDConfig()
CLIConfig = Annotated[OptimizerConfig, tyro.conf.arg(constructor=name_to_cfg)]
@dataclass
class Config:
optimizer: CLIConfig = AdamConfig(10.0, 10.0) it should show |
Sorry that was a bit unclear yeah. That edit was in reference to the previous request which you mentioned needed a rewrite of the argparse backend. What I meant was, in the scenario where we are using tyro.conf.ConsolidateSubcommandArgs and every Union has either a default or a choice passed in by the user via the CLI (so either adam is default, or e.g. the user passes |
I see, so the end goal is for both:
and
to work, right? Currently we can only choose between the first (default) or second (via If we retain the argparse backend, the low-level changes that would need to happen are:
Overall I think this is doable; at a high-level we already have the tree traversal scaffolding to make this happen. But the maintenance overhead does seem high. :/ |
Yes, that would be ideal. I understand it's much more involved than it seems, so right now I am using a more hacky approach where I'm capturing sys.argv checking it myself if the user passed a choice that should go to a union object and then calling I think the idea of propagating the doc for a union choice (mentioned at the start of #221 (comment)) is possibly separate? Is there a way to make the text passed to |
I think if you want to hack this in it should be doable, but making it the default behavior might also be complicated:
I think there's probably some better API / set of default behaviors for displaying or configuring the helptext headings, it's just a matter of balancing with complexity... does that answer your question? |
I understand, thanks. btw I think some of the CI will run in forks every time they sync, e.g. https://github.com/mirceamironenco/tyro/actions/runs/12707047668. adding |
Added! |
Thanks again and sorry for the wild-goose chase! If you are curious (and maybe someone else has a similar use-case): this branch implements a work-in-progress solution for the original problem (that works for the use-case presented above; it also changes how columns are formatted to be less restrictive with min height requirements). I still need to figure out how to show the user the helptext for subcommand choices and add group subtitles. The TL;DR of the approach is: rely on tyro's (Cannot promise this doesn't have bugs. Also links to specific file lines in the comment above might not be valid as I'm still working on this.) edit: Above solution could not deal with nested choices, and required installing from a different branch. The following is more robust and can be used as a patch on top of current tyro installation: https://gist.github.com/mirceamironenco/b3c6f0e7a6c44f4de78007bd5a133b6e |
Hi! I have the following use-case:
py tyro_ex.py --help
will force me to choose a subcommand (great):Once I choose an optimizer via a subcommand (
py tyro_ex.py cfg.optimizer:adam --help
), I can then manipulate (and get helptext for) the arguments of both the parent config, and the nested optimizer config:My question is, how to set a "default subcommand value" that can also possibly be changed? i.e. I'd like to be able to do something like:
and if the user invokes
py tyro_ex.py --help
, the behaviour+helptext would be identical to the second example above (py tyro_ex.py cfg.optimizer:adam --help
), but it would still be possible to choose a different optimizer config e.g.py tyro_ex.py cfg.optimizer:sgd --help
should adapt the choice. The above case (= AdamConfg()
) will still force me choose a subcommand (I assume due to the custom constructor). Things I've tried:tyro.conf.AvoidSubcommands
for both theAdamConfig | SGDConfig
union, or the return of the_constructor
. The constructors I'm working with are a bit more involved but having a solution even for this case would be great.sys.argv
to appendcfg.optimizer:adam
even when the user invokes onlypy tyro_ex.py --help
however I would then still need a way to show to the user passing--help
that thecfg.optimizer
is still a choice which can be modified.I don't have a hard requirement for how the default is delivered. Preferably it would be either in the
Config
dataclass or the custom constructor, but any working solution would be a good starting point. I'm usingconstructor_factory
since it's possible the choices are dynamically modified by the user beforetyro.cli
is invoked (think a registry system ofname: str -> OptimizerConfig
where someone adds a custom optimizer, and then invokes the cli entrypoint).I assume it should be clear from the docs already whether this is possible or not, but I wasn't able to make it work - I assumed it was possible since
py tyro_ex.py cfg.optimizer:adam --help
does such a good job of propagating the options 'post-command choice' that it would make sense for a default command setting to allow the same behavior. Maybe it would be possible to open the 'Discussions' feature on the repo for questions like this? Thanks!small additional question: is it possible to modify the title of a subcategory in the helptext? i.e. for
py tyro_ex.py cfg.optimizer:adam --help
as above, to have something like:edit:
The following also doesn't work:
I am still forced to choose an optimizer.
edit 2: Should this line say "... use tyro.confAvoidSubcommands[]"?
tyro/src/tyro/_cli.py
Line 286 in 1704284
The text was updated successfully, but these errors were encountered: