-
Notifications
You must be signed in to change notification settings - Fork 654
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 fastai upstream and downstream capacities for fastai>=2.4 and fastcore>=1.3.27 versions #678
Conversation
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.
Very nice! I just left a few nits :)
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.
Thank you very much for the PR! There are some important things to address, but overall is going in right direction 🚀
…e name from fastai_mixin
…gingface_hub into add-fastai-mixin
Hi, @osanseviero @muellerzr thank you for your kind reviews guys! All feedback was applied and tested. Main changes:
Will be here :D |
The failing tests are unrelated. They are flaky and likely not related to this PR, so don't worry about it |
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.
Looking good! I think it's in a good state to start adding some tests 😄
* fastai was made compatible with Python 3.10 just in the most recent version 3.5.6 released on April 1st. I would prefer to wait for the next release of fastai.
… test_fastai_integration.py
* Additionally, isort test_fastai_integration.py * Change the name from save_fastai_learner to _save_pretrained_fastai * isort __init__.py
* functions from_pretrained_fastai and push_to_hub_fastai were added to mixins.mdx
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.
Cool 🚀 Thanks for iterating on this
…rectory * By default learner.export saves learner to learner.path * Before we where saving the model in learner.path and them moving it to save_directory * Fix by changing learner.path to being equal to save_directory
* Additionally, makes optional that the pyproject.toml contains the fastai and fastcore versions * Will continue to throw an error if the toml library is not available * Will continue to thrown an error if the pyproject.toml specifies fastai or fastcore versions that are not supported by from_pretrained_fastai * This will allow to load fastai models from the Hub that were not necessarily uploaded using push_to_hub_fastai.
…and "requires" section
* This allows to eliminate the returns when checking for the versions of fastai and fastcore in the pyproject.toml
* This allows to eliminate the returns when checking for the versions of fastai and fastcore in the pyproject.toml
…gingface_hub into add-fastai-mixin
…into add-fastai-mixin
Congrats @omarespejel! 🚀 🚀 🚀 |
Thank you @osanseviero @adrinjalali @muellerzr @nateraw! For your reviews and feedback 🤗🥳 |
Sorry, I created a new PR (previous one: PR huggingface/huggingface_hub#416) I had a couple of mistakes with git, and seemed easier to create a new one. Thanks @muellerzr and @osanseviero for your comments!
What
fastai>=2.4
(link) andfastcore>=1.3.27
(link).Why
Learners
involves several changes and the fastai1 library has not been updated for over a year.2.0.6>= fastai <2.4
involve complex changes.Fastai
was updated due to what appear to be modifications to pytorch/serialization.py. For the sake of agility in our first release, I suggest releasing this version without supporting these previous versions.fastai>=2.4
versions work withfastcore>=1.3.27
version which is installed automatically whenfastai>=2.4
is installed.How?
file_download.py
checks for fastcore and fastai availability and versions.config.json
when pushed to hub.README.md
is automatically generated, if none, with thefastai
tag.Testing?
fastai>=2.4
versions work with the present code and with thefastcore==1.3.27
version.Next?
fastai <2.4
versions. I will ask on the fastai forums to assess how many users would require this support.