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

Harmonize parameters in list_models, list_datasets and list_spaces #1153

Open
Wauplin opened this issue Nov 3, 2022 · 1 comment
Open

Harmonize parameters in list_models, list_datasets and list_spaces #1153

Wauplin opened this issue Nov 3, 2022 · 1 comment

Comments

@Wauplin
Copy link
Contributor

Wauplin commented Nov 3, 2022

Feedback from #1146:
=> we are using both snake_case (fetch_config) and camelCase (securityStatus, cardData, lastModified) in the naming. This is surely done to match the backend naming but it makes huggingface_hub inconsistent.

Feeback from @Narsil (on slack - internal link):

  • list_models(filter=ModelFilter(filter=ModelSearchArguments().library.Transformers) feels weird to write
  • using direction was not obvious when using sort (list_models(sort="downloads")) (from @Wauplin : maybe using sort="-downloads") ?)
  • the sort options refers to all attributes of ModelInfo but all of them are not explicitly defined ("downloads" for example) => maybe we could enforce ModelInfo to define most up to date attributes ?
  • It felt weird to use a string "downloads" for sort, and an enum for filter
  • I would have imagined that the options as enums would all have been directly importable from the lib (not sure if feasible or if gotten dynamicaly from the hub)
  • I would have imagined able to use all strings everywhere (and be yelled at if the string was invalid with potential options).
  • I didn't need but expected more complex options like and or mecanism for filtering.

Feedback from @Wauplin :

  • initilializing ModelSearchArguments() or DatasetSearchArguments() takes a long time since it requires to first list all models/datasets from the Hub. Kinda niche problem but it feels weird to loop through all models before making a search request to the Hub. => maybe we could still have this but promote the string-based method ?

All of this might not be tackled in a single PR but I'm dropping them all at once since they are related to the same topic.

@Wauplin
Copy link
Contributor Author

Wauplin commented Dec 22, 2022

Feedback from @nbroad1881 (on slack - internal link):

  • When I do [x for x in list_models(search="bert") if x.tags is not None] I get an empty list which means they all have None for the tags => have to set full=True.
  • Example in docs is broken. Update from @osanseviero: When using list_models with no filter, you don't get the full results. When using a filter you do get all the information (including tags) => so example is not broken (but behavior is not intuitive)
  • the full keyword should probably be mentioned on this page

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

No branches or pull requests

1 participant