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

[FEATURE] Addition of missing typehints/docs #1989

Open
a-r-r-o-w opened this issue Oct 16, 2023 · 2 comments
Open

[FEATURE] Addition of missing typehints/docs #1989

a-r-r-o-w opened this issue Oct 16, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@a-r-r-o-w
Copy link
Contributor

This is not exactly a feature request, I guess, but hopefully still something along those lines?

Is your feature request related to a problem? Please describe.
Multiple files such as timm/models/mobilenetv3.py are missing type hints and docs for what the various parameters mean.

Describe the solution you'd like
Addition of docs where necessary and improvement in type hints.

Describe alternatives you've considered
.

Additional context
As someone who is trying to understand paper-to-code implementations of different models, I find it helpful to have descriptions of uncommon parameters. It's also helpful to have type hints on the return types and consumed parameters of functions, which really improve the behaviour of static type analysis tools in type-ahead suggestions made.

Is this something the team/community would be interested in having added? If so, I would love to try and contribute here. I'm trying to do something similar on the diffusers repository as well in my free time.

@a-r-r-o-w a-r-r-o-w added the enhancement New feature or request label Oct 16, 2023
@rwightman
Copy link
Collaborator

rwightman commented Oct 17, 2023

@a-r-r-o-w so yeah, there are a LOT of missing type hints and docstrings, some more recent models have some, most older ones do not.

I'm open to PRs, as it'll be nice to improve this. But, incorrect / misleading typing and comments are worse than none, so will be somewhat picky.

For docstrings, should try to adhere to Google style. No redundancy of type or default info from argument definitions in the docstring, as that's too difficult to maintain.

Example: https://github.com/huggingface/pytorch-image-models/blob/main/timm/models/_factory.py#L38

See also https://github.com/huggingface/pytorch-image-models/blob/main/CONTRIBUTING.md

@a-r-r-o-w
Copy link
Contributor Author

Got it @rwightman! Will try to keep the changes as simple as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants