Simpler support for new model libraries #482
Conversation
julien-c
requested review from
coyotte508,
osanseviero,
SBrandeis,
gary149 and
Wauplin
as code owners
| export const ALL_DISPLAY_MODEL_LIBRARY_KEYS = ALL_MODEL_LIBRARY_KEYS.filter( | ||
| (k) => !["doctr", "k2", "mindspore", "tensorflowtts"].includes(k) | ||
| ); |
There was a problem hiding this comment.
there was already this @osanseviero but i wrapped it inside the main dict
Note that i used an opt-in "filter: true" which seems friendlier to me
The elastic queries i will add in a second PR
There was a problem hiding this comment.
Awesome, thanks a lot!
There was a problem hiding this comment.
Was ALL_MODEL_LIBRARY_KEYS being used? They were still shown 🤔
There was a problem hiding this comment.
yes, looks like ALL_DISPLAY_MODEL_LIBRARY_KEYS (the filtered one) was only used in the suggestion dropdown in the GUI editor:
There was a problem hiding this comment.
Ok, we need a follow up PR so ALL_DISPLAY_MODEL_LIBRARY_KEYS is also used to control the filters at left of hf.co/models
|
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. |
| export const ALL_DISPLAY_MODEL_LIBRARY_KEYS = ALL_MODEL_LIBRARY_KEYS.filter( | ||
| (k) => !["doctr", "k2", "mindspore", "tensorflowtts"].includes(k) | ||
| ); |
There was a problem hiding this comment.
Awesome, thanks a lot!
| export const ALL_DISPLAY_MODEL_LIBRARY_KEYS = ALL_MODEL_LIBRARY_KEYS.filter( | ||
| (k) => !["doctr", "k2", "mindspore", "tensorflowtts"].includes(k) | ||
| ); |
There was a problem hiding this comment.
Was ALL_MODEL_LIBRARY_KEYS being used? They were still shown 🤔
There was a problem hiding this comment.
PR looks good to me but I don't think it closes the issue. In https://github.com/huggingface/moon-landing/issues/8791#issuecomment-1912339361 @osanseviero suggested to:
- be able to remove the library from the filter list (done in this PR)
- move the Elastic queries to have everything in a single file (and public) => still to be done or it's already decided that we don't do it?
| @@ -1,52 +1,333 @@ | |||
| import * as snippets from "./library-ui-elements"; | |||
There was a problem hiding this comment.
Let's rename the file to make it more explicit it only contains snippets maybe
|
i propose to add the elastic queries in a subsequent PR |
|
(cc @lhoestq also for dataset libraries) |
|
Second part is in #485, please review 🙏 |
Second part of #482 note to reviewers: I only expose the `filter` option because: - it's simpler: every library except for diffusers is using it. - i had typings issues/conflicts with the Chai library when defining a type that contains a property named `should`. cc @coyotte508. Rather than fightining the tooling, i decided to work around it
close this internal issue (ignore the unrelated README changes)