feat: Annotated based argparse, and auto completer inference#1614
feat: Annotated based argparse, and auto completer inference#1614KelvinChung2000 wants to merge 6 commits intopython-cmd2:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1614 +/- ##
==========================================
- Coverage 99.51% 99.40% -0.12%
==========================================
Files 21 22 +1
Lines 4758 5031 +273
==========================================
+ Hits 4735 5001 +266
- Misses 23 30 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I started working on this before I closed that PR. While this PR is still using LLMs, I have a much better understanding of what it is writing, since it is based on Python type processing, which I am much more familiar with than click. As mentioned, this code needs some more cleanup before it's ready for review; that's why this is a draft. However, I would like some feedback on the documentation and the example to make sure I haven't missed anything obvious. If you'd prefer to defer until it is fully ready, that's fine as well. |
|
I'm curious to see where this goes. I can't make any promises in advance, but it sounds like a potentially interesting feature. Though, I would prefer for all the tests to pass before I spend any time reviewing it. If the code isn't too complex so that it appears to integrate with the rest of If for some reason it doesn't immediately integrate well, there may be the possibility of creating a new open-source module that generates argparse argument parsers from type annotations. |
|
|
||
| _convert.__name__ = enum_class.__name__ | ||
| # Preserve the enum class for downstream consumers like tab completion. | ||
| _convert._cmd2_enum_class = enum_class |
There was a problem hiding this comment.
Need to fix fundamental type error causing all tests and mypy to fail:
cmd2/annotated.py:234:5: error: "Callable[[str], Enum]" has no attribute "_cmd2_enum_class" [attr-defined]There was a problem hiding this comment.
I am aware of this. I am working on the code's style right now. I will push once it is at a good enough state.
This page of code contains all the logic required to generate the argparser. While it is not too difficult to understand, I don't like the long if-else chain because it is not intuitive.
There was a problem hiding this comment.
I haven't look at this particular case, but since cmd2 now only supports Python 3.10 or newer, using the match statement can be a nice option for structural pattern matching with match/case to replace some long if/else chains.
There was a problem hiding this comment.
I originally considered something like a dict dispatch pattern:
def dis(type_to_resolve):
type_dict = {Enum: _reslove_enum}
return type_dict[type_to_resolve]But the things that return for the _resolve_* are too different, and are slowly moving toward more like filling in an "argparse specification," then having a function that takes in this specification and turns it into a parser.
I like this dict dispatch pattern because it is very easy to read and extend, but it might not be the right pattern here.
This is a full rework of #1612. Instead of wrapping Typer. We now extract the types and build the
argparseparser.I want to mark it as a draft for now, as some of the stuff will likely need a bit more cleanup. Please have a look at the documentation and example, and let me know if I missed anything obvious.