Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Discussion: download and upload models have a somewhat inconsistent API #47

Closed
osanseviero opened this issue May 20, 2021 · 6 comments · Fixed by #574
Closed

Discussion: download and upload models have a somewhat inconsistent API #47

osanseviero opened this issue May 20, 2021 · 6 comments · Fixed by #574
Assignees

Comments

@osanseviero
Copy link
Contributor

Hi all. Just a small-ish thing to discuss.

Methods that get information about/from repos take the full model id (e.g. hf_api.model_info("osanseviero/my-model") or snapshot_download("osanseviero/my-model)).

On the other hand, create_repo takes as two separate arguments the model name and the organization (if any). I wonder if this could lead to confusions in the future, and, if so, we should check if the name param has a / and determine if there's an org in there.

@nreimers
Copy link

nreimers commented Jun 22, 2021

I think this makes a lot of sense. Actually implemented it in the save_to_hub from sentence-transformers

Users are used to pull images like {org}/{name}, so it makes sense to have a push_to_hub("{org}/{name}")

With the current setup, with two parameters, users need to read the docs in order to know what there is a second parameter for the organization. And as we know: Users are not good in reading the docs ;)

@osanseviero
Copy link
Contributor Author

@LysandreJik @sgugger @julien-c in case you have any suggestions/ideas :)

We now have better error messages when this error happens, so it might not be such a big issue.

@LysandreJik
Copy link
Member

I wouldn't mind having org/name be interpreted as name, organization=org if specified, I think it would be coherent with the downloading methods.

@julien-c
Copy link
Member

Yes I'd be supportive of this as well

@severo
Copy link
Collaborator

severo commented Jun 22, 2021

Related: while working on the models' data in https://observablehq.com/@huggingface/kaggle-dataset-huggingface-modelhub, I have been surprised that some of the "modelId"s are not "user/model", but just "model" (I understand all of them are models provided by Hugging Face as the organization). eg: https://huggingface.co/bert-base-uncased

@LysandreJik
Copy link
Member

Yes, this is for historic purposes. The first models had no organization and were therefore at the root.

Unfortunately, for backwards compatibility purposes, it is now complicated to change the behavior.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants