-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
timm: add __all__ to __init__ #2399
Conversation
Does something like |
Unfortunately that doesn't work for mypy, flake8, ruff, or any other static style checker as it is dynamically generated. The maintenance burden depends on how often you update |
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. |
If doing this, I think rather than use the all approach, equally redundant/pedantic, but less error prone in terms of getting out of sync with futuer changes is the
|
probably worth mentioning, while I don't mind improving many aspects of typing, mypy compat... however, I probably will hit a limit where I won't agree with some changes mypy (esp strict) where verbosity, overhead outweights (my) perceived benefits |
Interesting. I've never seen anyone use this hack before, so I'm not sure if it's recommended, but I can confirm that it works. Let me know if you would like me to go with this solution instead.
If you're concerned about this, any of the above tools can be used to enforce this, so it will never get out of sync.
Completely understand. But note that this decision also affects other libraries as a result. In TorchGeo, we now have to add |
@rwightman what are the next steps for this PR? |
@adamjstewart can proceed with the |
Where do you see this? I see |
@adamjstewart I guess they did it inconsistently https://github.com/pytorch/pytorch/blob/main/torch/nn/__init__.py |
This fixes mypy errors on files like:
The error can be reproduced using: