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
feat: add type hints for param_store.py in params
module
#3148
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willtai thanks for the PR! Can you run make lint
locally, fix any linter warnings, then run make format
?
pyro/params/param_store.py
Outdated
@@ -4,10 +4,13 @@ | |||
import re | |||
import warnings | |||
import weakref | |||
from collections.abc import Iterator, KeysView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willtai I think you need to replace collections.abc.Iterator
with typing.Iterable
for tests to pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! However, I imported typing.Iterator
rather than typing.Iterable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Could you also remove the [mypy-pyro.params.*]
section from setup.cfg? We have been using setup.cfg as an ignore-list while we progressively add type hints to Pyro's codebase.
@willtai are you still working on this? Would it be okay if I take over this PR? |
Has anyone tried vscode's new copilot "brushes" feature to add type hints? I've bee having good luck just telling gpt-3.5-turbo to refactor my python code, and we may be able to automate much of this type hinting. |
Replaced by #3271 |
This PR adds type hints to the
params
module. This address this a section of this issue to add type hints.I intend to add more type hints wherever I can in future PRs.