search

brentyi / tyro

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

Rough Draft #115

Closed kschwab closed 3 months ago

kschwab commented 8 months ago

Hey @brentyi!

Here's a rough draft of what we covered in #60. I have to set this down for now but wanted to drop this here in case you could pull it across the finish line πŸ˜„

It mostly works. I haven't added tests etc. but did make sure all the existing tests pass. It should provide a decent feel for what we're trying to accomplish. There is still cleanup needed around required field handling and associated help text, lots of testing, etc.

One additional item I added was "HideNoneSubcommands". I think this would be really nice to have as well. For our use case, None is only intended as an invalid field that can be overridden (i.e., not used to invalidate an already valid object).

Mutual exclusion I took the stance of using defaults, there are probably better ways to enforce this.

Lastly, it seems with the current implementation most of this will need to be constrained to the field list generation. There may be better ways to accomplish this. I had issues with types getting interpreted differently if I tried to resolve it at a higher level (i.e., mainly field_list_from_callable).

kschwab commented 8 months ago

Also, I deferred converting cmd:None subcommands into flags (e.g. --cmd:None) and instead just left them as is.

codecov[bot] commented 8 months ago

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (3962ed8) 99.58% compared to head (5adcdb3) 98.66%.

Files Patch % Lines
src/tyro/_fields.py 42.85% 12 Missing :warning:
src/tyro/_arguments.py 25.00% 3 Missing :warning:
src/tyro/_docstrings.py 66.66% 2 Missing :warning:
src/tyro/_strings.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #115 +/- ## ========================================== - Coverage 99.58% 98.66% -0.92% ========================================== Files 24 24 Lines 1918 1952 +34 ========================================== + Hits 1910 1926 +16 - Misses 8 26 +18 ``` | [Flag](https://app.codecov.io/gh/brentyi/tyro/pull/115/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brent+Yi) | Coverage Ξ” | | |---|---|---| | [unittests](https://app.codecov.io/gh/brentyi/tyro/pull/115/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brent+Yi) | `98.66% <57.14%> (-0.92%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brent+Yi#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.