Use huggingface_hub dependency, remove inlined HF code #409
Conversation
|
Thanks ! We might be able to remove some of the hub dependencies thanks to this. |
jonashaag
force-pushed
the
huggingface-lib
branch
from
4d19fd9
to
8187c07
Compare
asteroid/utils/hub_utils.py
Outdated
| url = hf_bucket_url(model_id=model_id, filename=HF_WEIGHTS_NAME, revision=revision) | ||
| return hf_get_from_cache(url, cache_dir=get_cache_dir()) | ||
| url = huggingface_hub.hf_hub_url( | ||
| model_id, filename=huggingface_hub.file_download.PYTORCH_WEIGHTS_NAME, revision=revision |
There was a problem hiding this comment.
I can expose PYTORCH_WEIGHTS_NAME in the library's init.py so that you don't have to import from a nested file
There was a problem hiding this comment.
That would be great.
asteroid/utils/hub_utils.py
Outdated
| @@ -57,7 +46,9 @@ def cached_download(filename_or_url): | |||
| if os.path.isfile(filename_or_url): | |||
| return filename_or_url | |||
|
|
|||
| if urlparse(filename_or_url).scheme in ("http", "https"): | |||
| if filename_or_url.startswith(("http://", "https://")) and not filename_or_url.startswith( | |||
| huggingface_hub.file_download.HUGGINGFACE_CO_REPO_URL_BASE | |||
There was a problem hiding this comment.
I would just add a https://huggingface.co/ constant in this local file as this is unlikely to ever change:)
There was a problem hiding this comment.
(and I would add the prefix-stripping here, unless you feel strongly about it!)
There was a problem hiding this comment.
No strong feelings, we can do it here.
|
Let's address the comments from Julien and merge this? Your PR in the official repo has been merged right? So we can have the good pip deps? |
|
No, we need to move the URL stripping here, see huggingface/huggingface_hub#8. And create a PR for |
|
Sorry about the looong delay! I've opened #440 on top of this PR, to make code simpler and re-add the hf.co url prefix trimming. |
* Update to huggingface_hub>=0.0.2 * Feature: trim https://huggingface.co/ prefix
Refs #401
Switch to huggingface_hub and allow for https://huggingface.co/ prefixed URLs.
We can also merge without the URL change if @julien-c doesn't like it.
CI currently fails due to missing dependency bump