Improvements non git mixin #618
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bring up to speed
muellerzr
changed the base branch from
non-git-mixins
to
non-git-mixins-up-to-date
LysandreJik
approved these changes
Jan 22, 2022
There was a problem hiding this comment.
This is quite a nice improvement to the existing PR. Feel free to merge that PR in mine, and then let's discuss what is needed on it before we can merge.
What needs to be taken care of before we can merge in main, in my opinion:
- Taking care of the 5GB files and chunking them before uploading them. Ideally the size would be given manually to the method, similarly to how it's done in
datasets'push_to_hubmethod which already respects that paradigm (see Push to hub capabilities forDatasetandDatasetDictdatasets#3098) - I may be mistaken but I don't think progress bars are implemented yet for
upload_file - Very explicit documentation mentioning that for complex workflows,
Repositoryshould be used as it respects the git workflow
Anything else? Exciting to see progress on this!
Comment on lines
+1342
to
+1343
| if commit_message is not None: | ||
| headers["Commit-Summary"] = commit_message |
|
Nope! That should be about it for this PR. Will make the adjustment + merge, then will work towards the list |
…erzr/huggingface_hub into improvements-non-git-mixin
LysandreJik
pushed a commit
to LysandreJik/huggingface_hub
that referenced
this pull request
Mar 29, 2022
Add in commit_message + tests Co-authored-by: Lysandre <lysandre.debut@reseau.eseo.fr>
LysandreJik
pushed a commit
to LysandreJik/huggingface_hub
that referenced
this pull request
Apr 11, 2022
Add in commit_message + tests Co-authored-by: Lysandre <lysandre.debut@reseau.eseo.fr>
LysandreJik
pushed a commit
to LysandreJik/huggingface_hub
that referenced
this pull request
Apr 20, 2022
Add in commit_message + tests Co-authored-by: Lysandre <lysandre.debut@reseau.eseo.fr>
Wauplin
added a commit
that referenced
this pull request
Aug 17, 2022
* Propose a non-git mixin * Fix merge * Improvements non git mixin (#618) Add in commit_message + tests Co-authored-by: Lysandre <lysandre.debut@reseau.eseo.fr> * Big refactor * FIX typos in contributing.md * Remove redefined logger in HfApi.py * Use upload_folder in both mixins + some docstring * moved back logger to top of hf_api >< * space in documentation can be ambiguous * WIP started deprecation * deprecate skip lfs file and use Path * added decorator to deprecate specific arguments + unittests for it * simplified tests * proper decorators * fix docstring * hubmixin: fixed existing tests + add http one * unique repo names across tests * make push_to_hub_keras work + tests * logs are not overwritten in push_to_hub_keras * flake8 * refacto push_to_hub from mixin.save_pretrained * deprecate positional argument in version 0.12 * remove docstring for deprecated skip_lfs_files * delete old logs when uploading keras model to hub * remove TODO in tests * remove useless todo * Update src/huggingface_hub/hub_mixin.py * flake8 * remove un-explicit _generate_url helper * exclude some folders from flake8 * clean pr * move out tests fixing to other issue * introduce decorator to deprecate tests * docstring * fix pattern in expect_deprecation decorator * docstring * remove extras['ml'] integration * Apply suggestions from code review Co-authored-by: Omar Sanseviero <osanseviero@gmail.com> * use expect_deprecation decorator in test * remove caret in comment * explicit mocked model * delete unused and failing tett * optional path_in_repo for root * revert back token docstring * Update src/huggingface_hub/hf_api.py Co-authored-by: Lysandre Debut <lysandre@huggingface.co> Co-authored-by: Zachary Mueller <muellerzr@gmail.com> Co-authored-by: Wauplin <lucainp@gmail.com> Co-authored-by: Omar Sanseviero <osanseviero@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the start of getting the
improvements-non-git-mixinup to speed and worked towards its completion.Basic changes found here:
upload_filenow takes in acommit_message. This was part of the intended implementation, so trickled it up. This also allows for the back-ends to write custom messages for if something is a weight, tokenizer, etc, mimicking how Repository can do so already.test_hubmixinthat makes sure the timings for uploading a new file to a blank repo, and uploading a new file to a repo with fairly large files is about the same. Currently the fake repo uploads 5 .5mb files (locally this took 12 seconds to accomplish. If we want to lower that to one .5mb file that's okay)