-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add integration with HF Hub #7833
base: dev
Are you sure you want to change the base?
Conversation
try: | ||
from huggingface_hub import PyTorchModelHubMixin | ||
except ImportError: | ||
PyTorchModelHubMixin = DummyPyTorchModelHubMixin |
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.
Hi @qubvel Thanks for the contribution. We have our own optional_import function to do something similar.
try: | |
from huggingface_hub import PyTorchModelHubMixin | |
except ImportError: | |
PyTorchModelHubMixin = DummyPyTorchModelHubMixin | |
PyTorchModelHubMixin, has_hg_hub = optional_import("huggingface_hub", name="PyTorchModelHubMixin") | |
if not has_hg_hub: | |
PyTorchModelHubMixin = DummyPyTorchModelHubMixin # could put DummyPyTorchModelHubMixin definition here instead |
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.
optional_import
provides its own dummy class which raises an exception whenever a member is requested, but this doesn't have __init_subclass__
so your version is needed.
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.
ok, great, I will update it!
Hi @qubvel Thanks again. Please double check places like this in setup.cfg have the updated information as your code mentions needed |
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.
Flake8 is correct in that this file is missing the license header that's in all our other source files.
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.
got it 👍
@ericspod thanks for the review, I will address the comments! At the moment it's a raw draft to know how you feel about this integration in general. Any comments on how to make it smooth are appreciated! I'm not sure I got the point here, can you provide more details?
|
Description
Hi! Amazing repo!
I made a draft with PytorchModelHubMixin for a few models in the repo. This mixin adds a few methods to make it possible to save/load models and share them with the HF hub.
Here is a short example:
I believe the repo and community may benefit from sharing pretrained models built with this framework!
Let me know if it aligns with your roadmap, I will be happy to help you with implementation or to complete the PR!
Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.