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

Document more list repos behavior #1823

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Nov 14, 2023

Related to #1814.

This PR updates the documentation of ModelInfo, DatasetInfo, SpaceInfo and RepoSiblings to let the users know they should not expect all attributes to exist. @ademait could you have a look and let me know what you think? I kept the wording a bit vague on purpose as I don't want to be too specific on things that we might change server-side in the future (all those objects are built in a way that is future-compatible, on purpose). (EDIT: I'm not saying that we will remove attributes in the future, but we might potentially add news ones and/or improve default behaviors).

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 14, 2023

The documentation is not available anymore as the PR was closed or merged.

@ademait
Copy link
Contributor

ademait commented Nov 14, 2023

Hi @Wauplin , thanks for addressing the issue! I guess know it is more clear 🙌

@Wauplin
Copy link
Contributor Author

Wauplin commented Nov 14, 2023

Good, thanks for the feedback @ademait :)
A final review from the team and we'll be good to merge.

src/huggingface_hub/hf_api.py Outdated Show resolved Hide resolved
Comment on lines +504 to +505
In general, the more specific the query, the more information is returned. On the contrary, when listing models
using [`list_models`] only a subset of the attributes are returned.
Copy link
Member

Choose a reason for hiding this comment

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

Not really seeing how mentioning list_models is connected to ModelInfo. Are you maybe trying to compare their usage (broad model search vs more narrow and specific model details)?

Suggested change
In general, the more specific the query, the more information is returned. On the contrary, when listing models
using [`list_models`] only a subset of the attributes are returned.
In general, the more specific the query, the more information is returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ModelInfo is the class returned by list_models (get info from many) and model_info (get info from one) but is not really meant to be used/instantiated outside of this scope. That's why I took the liberty to tie the documentation of ModelInfo to list_models.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, thanks for the clarification. In that case, feel free to ignore this comment! :)

Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
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.

Thank you @Wauplin

@Wauplin
Copy link
Contributor Author

Wauplin commented Nov 15, 2023

Thanks for the reviews! ❤️

@Wauplin Wauplin merged commit 975b931 into main Nov 15, 2023
12 of 16 checks passed
@Wauplin Wauplin deleted the 1814-warn-about-list-models-behavior branch November 15, 2023 18:08
@Pierrci
Copy link
Member

Pierrci commented Nov 15, 2023

Thanks @Wauplin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants