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

Using Python type hints #2550

Open
4 of 23 tasks
jpchen opened this issue Jul 6, 2020 · 7 comments
Open
4 of 23 tasks

Using Python type hints #2550

jpchen opened this issue Jul 6, 2020 · 7 comments

Comments

@jpchen
Copy link
Member

jpchen commented Jul 6, 2020

Since Pyro supports exclusively Python 3, we should look into adding type hints. This has a few advantages:

  • pep8 linters can check for type errors
  • easier for a reader to understand function signatures. useful for eg funsor where multiple dispatching is prevalent.
  • sphinx docs can autogenerate types of args
  • IDE static analysis tools will work (better)

e.g.

from typing import List, Optional

def foo(bar: List, baz: Optional[List] = None) -> List:
    return bar

Add type hints to modules

Additional functionality for type hints

  • Give warnings on #type ignore in codebase
    Grey might be a good idea for finding #type ignores
  • Give warnings on modules skipped by mypy
@fritzo
Copy link
Member

fritzo commented Aug 18, 2020

I guess we should add a mypy test stage

@fritzo
Copy link
Member

fritzo commented Apr 28, 2021

This would also allow us to remove sphinx type annotations, if we were to use https://pypi.org/project/sphinx-autodoc-typehints/

@kamathhrishi
Copy link
Contributor

kamathhrishi commented Jun 1, 2021

Can I work on this issue? Adding type hints to the existing codebase? Maybe not the entire codebase in a PR, but at least some sections of it. Then subsequent PR's could cover the codebase.

@fritzo
Copy link
Member

fritzo commented Jun 1, 2021

Hi @kamathhrishi, sure, we'd appreciate any help! I especially like the idea of a bunch of small easy-to-review PRs that add type hints to one module at a time.

@kamathhrishi
Copy link
Contributor

kamathhrishi commented Jun 1, 2021

Thank you @fritzo. I have made a PR for optim module. Review it and let me know. I am new to the codebase and would like to take this as a starting step.

@fritzo
Copy link
Member

fritzo commented Mar 12, 2022

It looks like we should also add py.types stub files to various directories to avoid errors like

infer.py:4: error: Skipping analyzing "pyro": module is installed, but missing library stubs or py.typed marker
infer.py:4: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
infer.py:5: error: Skipping analyzing "pyro.distributions": module is installed, but missing library stubs or py.typed marker
infer.py:8: error: Skipping analyzing "pyro.infer": module is installed, but missing library stubs or py.typed marker
infer.py:9: error: Skipping analyzing "pyro.infer.autoguide": module is installed, but missing library stubs or py.typed marker
infer.py:10: error: Skipping analyzing "pyro.infer.reparam": module is installed, but missing library stubs or py.typed marker
infer.py:11: error: Skipping analyzing "pyro.optim": module is installed, but missing library stubs or py.typed marker

@willtai
Copy link

willtai commented Oct 22, 2022

Can I take a look at parts of this? I can open a PR for params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants