-
Notifications
You must be signed in to change notification settings - Fork 493
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
Turn command import into optional #936
Conversation
The documentation is not available anymore as the PR was closed or merged. |
I am personally not in favor of this - basically some commands will not show depending on whether you have installed a package or not? Ideally you would want to see all possible commands, with errors raised if you miss a package for one. |
@michaelbenayoun WDYT? @fxmarty What about my initial suggestion on not setting any default subcommand, and add them by register? Like registering onnx, tflite and onnxruntime in a |
I agree with @fxmarty, what about doing this instead: each |
Can I merge this one? @michaelbenayoun @fxmarty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (as long as the tests pass)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks a lot!
What does this PR do?
as per title.