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

Prepare for 0.14 #1381

Merged
merged 18 commits into from
Mar 23, 2023
Merged

Prepare for 0.14 #1381

merged 18 commits into from
Mar 23, 2023

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Mar 8, 2023

Remove deprecated methods:

  • set_access_token
  • unset_access_token
  • read_from_credential_store
  • write_to_credential_store
  • erase_from_credential_store

All of those are related to git credentials management. The preferred methods are not set_git_credentials and unset_git_credential. Their main advantage is to respect the user preferences in term of git credential helper instead of assuming git-credential-store should be used. Read #1051 and #1138 for more details.

I also took the opportunity of this PR to refactor some tests that were using set_access_token.


set_access_token was use in other HF repos. However, it was only for testing purposes and seemed to be legacy copy-pasted code (i.e. I don't except a lot of third-party libraries to use it). Here are the PRs to fix that:

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 8, 2023

The documentation is not available anymore as the PR was closed or merged.

@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@8e7365d). Click here to learn what that means.
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head dea55e1 differs from pull request most recent head fc3611f. Consider uploading reports for the commit fc3611f to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1381   +/-   ##
=======================================
  Coverage        ?   83.78%           
=======================================
  Files           ?       48           
  Lines           ?     4903           
  Branches        ?        0           
=======================================
  Hits            ?     4108           
  Misses          ?      795           
  Partials        ?        0           
Impacted Files Coverage Δ
src/huggingface_hub/_commit_api.py 93.67% <ø> (ø)
src/huggingface_hub/commands/user.py 0.00% <ø> (ø)
src/huggingface_hub/hf_api.py 88.85% <ø> (ø)
src/huggingface_hub/__init__.py 75.75% <100.00%> (ø)
src/huggingface_hub/_login.py 25.00% <100.00%> (ø)
src/huggingface_hub/utils/__init__.py 100.00% <100.00%> (ø)
src/huggingface_hub/utils/_git_credential.py 91.30% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Wauplin Wauplin marked this pull request as ready for review March 9, 2023 10:11
@Wauplin Wauplin changed the title [WIP] Prepare for 0.14 Prepare for 0.14 Mar 9, 2023
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Yes, looks good to me! I've quickly verified and it seems like this won't impact transformers. Let's remember to still launch the full test suite in downstream libraries to ensure nothing breaks.

I'll do so for transformres once this PR is merged in!

@Wauplin
Copy link
Contributor Author

Wauplin commented Mar 23, 2023

Thanks for the review! As said in the PR description, I tried to take care of the breaking changes in transformers, dataset and hffs pipelines. But yes, definitely a good idea to merge it and try for real :)

(I'm merging it btw)

@Wauplin Wauplin merged commit ce494a2 into main Mar 23, 2023
@Wauplin Wauplin deleted the prepare-for-0.14 branch March 23, 2023 14:46
@LysandreJik
Copy link
Member

Great, will give it a try now and report here ASAP

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.

3 participants