Add typing support - Githubissues

matthewfeickert commented 3 years ago

Suggestion: add from __future__ import annotations to all your files (using isort or reorder_python_imports), make sure you are using mypy 0.800, make sure you have your minimum python version for mypy set to 3.7, and then use things like this in your types!

mixed_list: list[str | int] = [1, "hi", 2]

:)

Originally posted by @henryiii in https://github.com/scikit-hep/pyhf/issues/1272#issuecomment-766285638

kratsg commented 3 years ago

See #948

matthewfeickert commented 3 years ago

At SciPy 2021 @obi1kenobi and @ColCarroll gave a great talk in the maintainers track on typing_copilot. This seems like the way forward for pyhf!

We can incrementally ratchet down the strictness.

For reference, c.f. https://github.com/arviz-devs/arviz/pull/1528

I think though a great endorsement for it is Colin's comment in the Q&A

I can't stress enough how little I care about type hinting compared to how much I care about MCMC!

Sounds like me. :)

cc @alexander-held

kratsg commented 3 years ago

I'm running this on feat/typing now and will rebase and update that branch.

$ typing_copilot init
typing_copilot v0.6.0

Running mypy once with laxest settings to establish a baseline. Please wait...

Collecting mypy errors from strictest check configuration. Please wait...

Strict run completed and uncovered 1742 mypy errors. Building the strictest mypy config such that all configured mypy checks still pass...

> Mypy was unable to find type hints for some 3rd party modules, configuring mypy to ignore them.
    More info: https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
    Affected modules: ['iminuit', 'numpy', 'papermill', 'scipy', 'scrapbook', 'setuptools', 'tensorflow', 'uproot']

> Constructed 116 mypy error suppression rules across 37 modules.

Config generated (232 lines). Verifying the last few mypy settings and validating that the new configuration does not produce mypy errors. Please wait...

Validation complete. Your mypy.ini file has been updated. Happy type-safe coding!

obi1kenobi commented 3 years ago

I'm running this on feat/typing now and will rebase and update that branch.

Awesome! Thanks for checking out typing_copilot, and please feel free to open issues for any rough edges you might run into.

I noticed that the top-level mypy.ini on the feat/typing branch had some extra flags on top of what typing_copilot generates on its own. This is something I'm hoping to add proper support for as part of https://github.com/obi1kenobi/typing_copilot/issues/11

If you or anyone else is interested in following development & giving feedback, or even contributing to typing_copilot, please feel free to jump in on that issue 🙌

kratsg commented 3 years ago

I noticed that the top-level mypy.ini on the feat/typing branch had some extra flags on top of what typing_copilot generates on its own. This is something I'm hoping to add proper support for as part of obi1kenobi/typing_copilot#11

This was something I was going to mention. We did have some extra flags. What's a little annoying atm is typing_copilot was ignoring the src/ flag so it was picking up top-level *.py files i have littered around.

obi1kenobi commented 3 years ago

Yup, it's quite unfortunate and yet somewhat nontrivial to fix. Does the "put your desired mypy global flags in pyproject.toml" solution from https://github.com/obi1kenobi/typing_copilot/issues/11 sound like it could fix that point of friction?

obi1kenobi commented 3 years ago

I just published a pre-release of typing_copilot that supports setting up custom mypy global configuration via the project's pyproject.toml file. If you have a sec to try it out and give me feedback, I'd really appreciate it!

Here's what you could do:

pip install typing_copilot==0.7.0.dev2
Add the following section to the pyproject.toml file, reflecting the additional mypy settings you had previously added by hand:
```
[tool-typing_copilot.mypy_global_config]
pretty = "True"
files = "src"
```
Then, run typing_copilot normally -- ideally, try both the init and tighten commands and let me know if the output was satisfactory.

Thanks for looking into typing_copilot! Good luck and please let me know how it goes.

EDIT: Released 0.7.0.dev2 with a couple of minor bugfixes, it's the best version to use to try this out.

kratsg commented 2 years ago

See #1934 for first.

scikit-hep / pyhf

Add typing support #1284