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

Add typing support #1284

Open
matthewfeickert opened this issue Feb 2, 2021 · 8 comments · May be fixed by #948
Open

Add typing support #1284

matthewfeickert opened this issue Feb 2, 2021 · 8 comments · May be fixed by #948
Labels
feat/enhancement New feature or request SciPy Conference Resulted from discussions at a SciPy conference

Comments

@matthewfeickert
Copy link
Member

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 #1272 (comment)

@matthewfeickert matthewfeickert added the feat/enhancement New feature or request label Feb 2, 2021
@kratsg
Copy link
Contributor

kratsg commented Feb 3, 2021

See #948

@matthewfeickert matthewfeickert linked a pull request Feb 3, 2021 that will close this issue
4 tasks
@matthewfeickert matthewfeickert added the SciPy Conference Resulted from discussions at a SciPy conference label Jul 16, 2021
@matthewfeickert
Copy link
Member Author

matthewfeickert commented Jul 16, 2021

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. arviz-devs/arviz#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
Copy link
Contributor

kratsg commented Jul 16, 2021

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
Copy link

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 obi1kenobi/typing_copilot#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
Copy link
Contributor

kratsg commented Jul 16, 2021

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
Copy link

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

@obi1kenobi
Copy link

obi1kenobi commented Aug 6, 2021

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
Copy link
Contributor

kratsg commented Aug 12, 2022

See #1934 for first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat/enhancement New feature or request SciPy Conference Resulted from discussions at a SciPy conference
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

3 participants