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

[HfApi] Fix sorting properties in list_models(), list_datasets() and list_spaces() #2655

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

hanouticelina
Copy link
Contributor

This PR fixes a bug reported in an internal slack message where the created_at field in ModelInfo does not work as a valid sort parameter for listing models via the api. the correct parameter should be createdAt rather than created_at. The same issue applies to the trending_score field.
I’ve restricted the list of possible sort values in the docstring to match those defined on the API side, as seen here (private link). also, I'm not sure if the sorting feature is widely used, but I've added some tests just in case!

@hanouticelina hanouticelina requested a review from Wauplin November 4, 2024 17:00
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks @hanouticelina !

Comment on lines +1914 to +1922
params["sort"] = (
"lastModified"
if sort == "last_modified"
else "trendingScore"
if sort == "trending_score"
else "createdAt"
if sort == "created_at"
else sort
)
Copy link
Contributor

Choose a reason for hiding this comment

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

if it ever becomes a problem again in the future we might explore having a "snake_case to camelCase" helper to fix it once and for all. Ok to keep it like this for now (at least things are clearly documented and tested).

Copy link
Member

Choose a reason for hiding this comment

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

beware, we like inconsistency in the hub team, we might use a snake case just to trick you! 🤪

@hanouticelina hanouticelina merged commit 9fb6a8c into main Nov 5, 2024
17 checks passed
@hanouticelina hanouticelina deleted the fix-list-models-sorting-properties branch November 5, 2024 08:58
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.

4 participants