Rough Draft #115
Closed
Rough Draft #115
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kschwab
commented
Jan 12, 2024
There was a problem hiding this comment.
I would suggest breaking this out of the PR and pulling it into main. This adds support for Sphinx autodoc comment strings. Currently it will insert a newline into the docstring formatting without this.
|
Also, I deferred converting |
Codecov ReportAttention:
Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
brentyi
force-pushed
the
main
branch
2 times, most recently
from
b9c0552
to
6bef529
Compare
brentyi
added a commit
that referenced
this pull request
Jan 18, 2024
brentyi
force-pushed
the
main
branch
4 times, most recently
from
33c954e
to
ed9e5e4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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,
Noneis 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).