Add missing ModelOutput subclass return type hints#41219
Add missing ModelOutput subclass return type hints#41219vasqu merged 3 commits intohuggingface:mainfrom
Conversation
|
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. |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: dac, deepseek_vl, deepseek_vl_hybrid, flava, grounding_dino, janus, maskformer, mm_grounding_dino, tvp, udop |
|
I fixed the merge conflicts. I totally forgot about this PR, but it should still be valuable to improve the typings somewhat.
|
vasqu
left a comment
There was a problem hiding this comment.
LGTM, I'm just wondering whether you only want to target the base model with this PR / how consistent this needs to be, e.g.
has no output type|
@vasqu The script I wrote to check the type hints indeed only checked the base models, also because Sentence Transformers will in the future rely on the type hints of the base models. Extending it to the non-base classes would be nice, but I don't have the bandwidth for that now, as I'm aiming to get #42564 ready before the full v5.
|
|
Gotcha, makes sense then. Let's merge, having more typing is better than nothing in the first place |
* Add missing ModelOutput subclass return type hints * Fix incorrect type hint: FlavaOutput (nn.Module subclass) -> FlavaModelOutput
What does this PR do?
This PR adds missing ModelOutput subclass return type hints. I'm running an analysis on the Model classes to see what kind of outputs are used for the various modalities, and I noticed a handful of architectures were missing them.
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@ArthurZucker