Skip to content
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

[MRG] add CLI for convert #286

Merged
merged 4 commits into from
Apr 21, 2021
Merged

[MRG] add CLI for convert #286

merged 4 commits into from
Apr 21, 2021

Conversation

dixing0908
Copy link
Contributor

@dixing0908 dixing0908 commented Apr 11, 2021

Closes #248

What has been done to verify that this works as intended?

This is a cli command for convert component.

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to documentation?

Before submitting this PR, please make sure you have:

  • run python -m pytest tests/ and confirmed all checks still pass.
  • verified that any code or assets from external sources are properly credited.

@HuaizhengZhang HuaizhengZhang changed the title ✨ add CLI for convert [WIP] ✨ add CLI for convert Apr 12, 2021
@univerone
Copy link
Collaborator

You can refer to #288 to get model object from its ID, @STALLAAAA

@dixing0908 dixing0908 changed the title [WIP] ✨ add CLI for convert [MRG] add CLI for convert Apr 15, 2021

@app.command('convert')
def convert(
id: str = typer.Option(..., '-i', '--id', help='ID of model.'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to add more argument, such as:

  • the model format use want to convert this model into, you can the use convert function
  • the saved path of the converted model
  • whether to register the generated model into model hub

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add 2 new arguments for savepath and register. I will add each specified convert after I make a revise to its function in the next PR.

@dixing0908 dixing0908 changed the title [MRG] add CLI for convert [WIP] add CLI for convert Apr 20, 2021
:fix: add 2 arguments of convert cli

:fix: make 'generate_model_family' a public function

:fix:convert cli: provide args:
     1.  the saved path of the converted model
     2.  whether to register the generated model into model hub
@dixing0908 dixing0908 changed the title [WIP] add CLI for convert [MRG] add CLI for convert Apr 21, 2021
@HuaizhengZhang HuaizhengZhang merged commit 72fe5ac into cap-ntu:master Apr 21, 2021
@dixing0908 dixing0908 deleted the dev branch April 21, 2021 07:50
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve convert function
3 participants