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

Fix test test_list_models_search #1106

Merged
merged 1 commit into from
Oct 10, 2022
Merged

Fix test test_list_models_search #1106

merged 1 commit into from
Oct 10, 2022

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Oct 10, 2022

This test is not ideal anyway as we are more testing the Hub behavior than huggingface_hub but I guess it is still a good test to keep for now. Hub search rules have recently changed to include more than the model id. Previous implementation of the test was checking that all returned models contained bert in there name which is not necesarly the case anymore.

A ok-ish solution is to test only the first 10 models that are returned and assume the search is not completely broken if it works for those ones :)

@Wauplin Wauplin requested review from LysandreJik and osanseviero and removed request for LysandreJik October 10, 2022 15:31
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 10, 2022

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

Copy link
Member

@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.

Sounds good as a temporal fix, thanks!

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.

Fair!

@Wauplin Wauplin merged commit 6415cd1 into main Oct 10, 2022
@Wauplin Wauplin deleted the fix-list-models-test branch October 10, 2022 19:41
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

4 participants