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

Bring consistency to download and upload APIs #574

Merged
merged 13 commits into from
Jan 5, 2022

Conversation

muellerzr
Copy link
Contributor

Closes #47

This PR aims to bring some consistency in how the api can be used, by letting a user both pass in a specified organization as well as using organization/model_name for functions revolving around uploading and downloading

@muellerzr muellerzr requested a review from osanseviero January 4, 2022 18:23
@muellerzr muellerzr marked this pull request as ready for review January 4, 2022 18:25
@muellerzr
Copy link
Contributor Author

@osanseviero mostly interested on if you think I missed any. There weren't many instances of this happening from a user perspective I could see, but one may have trickled past me

Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Overall looks good. I think you cover all the cases

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.

Agree with @osanseviero's comment, otherwise this looks good!

@muellerzr
Copy link
Contributor Author

Based on advice from @LysandreJik I forgo that logic for a complete refactor, making use of the repo_type_and_id_from_hf_id functionality, since we already have a function that can check if it's a valid namespace.

@muellerzr muellerzr requested a review from LysandreJik January 5, 2022 13:39
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.

Looks good to me!

Comment on lines +817 to +820
raise ValueError(
f"""Passed `organization` and `name` organization are not the same ({organization}, {checked_name[1]}).
Please either include the organization in only `name` or the `organization` parameter, such as `api.create_repo({checked_name[0]}, organization={organization})` or `api.create_repo({checked_name[1]}/{checked_name[2]})`"""
)
Copy link
Member

Choose a reason for hiding this comment

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

Nice

@muellerzr muellerzr merged commit bb6fec0 into huggingface:main Jan 5, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discussion: download and upload models have a somewhat inconsistent API
3 participants