espnet / espnet_model_zoo

ESPnet Model Zoo
Apache License 2.0
241 stars 39 forks source link

Huggingface compatibility #39

Closed Fhrozen closed 3 years ago

Fhrozen commented 3 years ago

This PR follows the recommendations did at: https://github.com/espnet/espnet/pull/3326

codecov-commenter commented 3 years ago

Codecov Report

Merging #39 (559e16a) into master (00beed6) will increase coverage by 6.43%. The diff coverage is 96.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage   53.45%   59.89%   +6.43%     
==========================================
  Files           2        3       +1     
  Lines         318      374      +56     
==========================================
+ Hits          170      224      +54     
- Misses        148      150       +2     
Impacted Files Coverage Δ
espnet_model_zoo/huggingface.py 96.00% <96.00%> (ø)
espnet_model_zoo/downloader.py 85.85% <100.00%> (+0.42%) :arrow_up:

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 00beed6...559e16a. Read the comment docs.

kamo-naoyuki commented 3 years ago

Why I prepared table.csv system is

I concluded that it's better to keep this system even if using huggingface. If not, there are no reason to use espnet_model_zoo repository.

Probably, it's little bit bothering for you to implement this feature as my desired shape, so can I take over this PR?

Fhrozen commented 3 years ago

@kan-bayashi

Fhrozen commented 3 years ago

@sw005320 @kamo-naoyuki @kan-bayashi any comment in this PR?

kan-bayashi commented 3 years ago

LGTM @Fhrozen why don't you answer Kamo-san's suggestion?

Fhrozen commented 3 years ago

@kan-bayashi a sure, @kamo-naoyuki , please you can take over this PR. I am just wondering because I am waiting this to continue with espnet/espnet#3326.

kamo-naoyuki commented 3 years ago

44