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

Add Huggingface-cli usage #4027

Merged
merged 17 commits into from
Feb 19, 2022
Merged

Add Huggingface-cli usage #4027

merged 17 commits into from
Feb 19, 2022

Conversation

karthik19967829
Copy link
Contributor

@karthik19967829 karthik19967829 commented Feb 3, 2022

This PR adds Huggingface-cli usage to upload models to Huggingface Hub

@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #4027 (354b981) into master (a3e1543) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4027   +/-   ##
=======================================
  Coverage   80.94%   80.94%           
=======================================
  Files         435      435           
  Lines       37429    37429           
=======================================
  Hits        30297    30297           
  Misses       7132     7132           
Flag Coverage Δ
test_integration_espnet1 67.13% <ø> (ø)
test_integration_espnet2 52.44% <ø> (ø)
test_python 66.61% <ø> (ø)
test_utils 24.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3e1543...354b981. Read the comment docs.

@sw005320 sw005320 requested a review from ftshijt February 3, 2022 12:52
@sw005320 sw005320 added this to the v.0.10.6 milestone Feb 3, 2022
Copy link
Collaborator

@ftshijt ftshijt left a comment

Choose a reason for hiding this comment

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

Many thanks!

CONTRIBUTING.md Outdated Show resolved Hide resolved
@sw005320
Copy link
Contributor

sw005320 commented Feb 4, 2022

I think we can remove the zenodo description (or at least emphasize that this is deprecated and we can move this part in the later part).

@ftshijt
Copy link
Collaborator

ftshijt commented Feb 4, 2022

Make sense, maybe just add a note (Not recommended) or something. It may still be interested by someone, so leaving it seems OK

@kan-bayashi kan-bayashi modified the milestones: v.0.10.6, v.0.10.7 Feb 8, 2022
@mergify mergify bot added the ESPnet2 label Feb 14, 2022
@karthik19967829
Copy link
Contributor Author

I think we can remove the zenodo description (or at least emphasize that this is deprecated and we can move this part in the later part).

Yes this makes sense, I have added this info next to the zenodo description in the doc

@@ -1478,7 +1478,7 @@ fi
if ! "${skip_upload_hf}"; then
if [ ${stage} -le 16 ] && [ ${stop_stage} -ge 16 ]; then
[ -z "${hf_repo}" ] && \
log "ERROR: You need to setup the variable hf_repo with the name of the repository located at HuggingFace" && \
log "ERROR: You need to setup the variable hf_repo with the name of the repository located at HuggingFace, follow the following steps described here https://github.com/espnet/espnet/pull/4027/files#diff-b9f381af6d3ca8c14a68dca2a24de815830013fd69a3fe6a113e108c7aaf0d2dR1490-R1505" && \
Copy link
Collaborator

Choose a reason for hiding this comment

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

The link here is not the correct link to file, but the pull info. Can you fix it as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure , updated with a permanent link

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you make it not stick to a specific commit? It won't reflect future changes with a fixed commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey in the latest commit the link is updated as

To upload models using Huggingface-cli follow the following steps:
, if this is not the permalink could guide me how to get it ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the link

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
karthik19967829 and others added 9 commits February 14, 2022 15:33
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
Co-authored-by: Shinji Watanabe <sw005320@gmail.com>
@ftshijt
Copy link
Collaborator

ftshijt commented Feb 15, 2022

LGTM. I will merge it after passing the CI tests

@mergify mergify bot added the ESPnet1 label Feb 15, 2022
@mergify mergify bot added the ASR Automatic speech recogntion label Feb 15, 2022
@karthik19967829
Copy link
Contributor Author

@ftshijt if I apply black locally, it says says no file to be reformatted. How to fix this CI error?

@sw005320
Copy link
Contributor

can you check that the black version was correct?
@roshansh-cmu had a similar issue recently. @roshansh-cmu, can you give us some advice?

@roshansh-cmu
Copy link
Contributor

@karthik19967829 @sw005320 - I recommend creating an environment with python=3.6 or 3.7, and installing all the CI tools there. Install black with python=3.6/3.7 and redo the black formatting. That worked for me

@karthik19967829
Copy link
Contributor Author

@karthik19967829 @sw005320 - I recommend creating an environment with python=3.6 or 3.7, and installing all the CI tools there. Install black with python=3.6/3.7 and redo the black formatting. That worked for me

Thank you this helped .

@ftshijt
Copy link
Collaborator

ftshijt commented Feb 19, 2022

Many thanks for the contribution!

@ftshijt ftshijt merged commit d4eb886 into espnet:master Feb 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants