Add mypy configuration file that ignores existing errors on a per-file and per-error basis #2424

namurphy · 2023-12-21T20:09:16Z

Description

The PR adds an initial mypy configuration file, mypy.ini. To get it to pass, I set it to ~~ignore a whole bunch of files~~ ignore specific error codes in a bunch of files.

Motivation and context

We currently have no method for checking that our type hint annotations are accurate. Enabling the mypy static type checker will fill this gap in our continuous integration testing.

Prior to #2421 and this PR, running mypy plasmapy led to 719 errors, of which ∼200 are from untyped packages like Astropy and SciPy. Rather than fixing all of these errors at once, I wanted to add a configuration for mypy that passes, even if it means that we ignore errors in a whole bunch of files. After merging this, we can gradually go through PlasmaPy file-by-file and error code by error code to get mypy to pass in strict mode. For errors that are difficult to deal with we could add # type: ignore[specific-error-code] comments so that the rest of the file gets type checked.

Related issues

Closes #268. Depends on #2421.

codecov · 2023-12-21T20:18:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cceb9ca) 96.92% compared to head (a3bcf0c) 96.91%.
Report is 1 commits behind head on main.

❗ Current head a3bcf0c differs from pull request most recent head 9295e10. Consider uploading reports for the commit 9295e10 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2424      +/-   ##
==========================================
- Coverage   96.92%   96.91%   -0.01%     
==========================================
  Files         104      104              
  Lines        9127     9122       -5     
==========================================
- Hits         8846     8841       -5     
  Misses        281      281

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Setting up the mypy configuration in pyproject.toml was giving me a headache when I was trying out how to exclude files.

namurphy · 2023-12-21T22:44:55Z

I just got mypy to pass for the first time in continuous integration ever on PlasmaPy, and all I needed to do was set it to completely ignore 79 files. 🥲 Oh well...it's a starting point!

namurphy · 2023-12-28T21:27:58Z

The number of files that aren't passing mypy went down after #2421, as expected!

.pre-commit-config.yaml

namurphy · 2023-12-29T01:56:07Z

pyproject.toml

+  "pandas-stubs >= 1.5.0.221010",
  "pre-commit >= 3.0.0",
  "pytest-regressions >= 2.3.1",
  "pytest-xdist >= 3.0.2",
  "tomli >= 2.0.1",
  "tox >= 4.3.1",
+  "types-requests >= 2.27.1",
+  "types-tqdm >= 4.66.0.3",


For packages that don't include type hints themselves, there sometimes exists stub packages that contain typing information. We might want to separate these out later on so that they don't get installed every time we run tests.

Is it worth adding a line comment on these packages explaining what they are (type information for the associated package)?

…e-mypy

Strangely, I was getting different results from mypy using a pre-commit hook vs running it locally. I think it's probably due to running different versio of mypy.

pheuer

Looks good - shouldn't really change much for users/developers until added to CI, except for the few new minor added requirements.

One note - presumably between now and when CI is turned on, more changes will be made to the code and it's possible the list of exception files will change and need to be updated in the PR that turns on CI?

pheuer · 2023-12-29T20:32:33Z

mypy.ini

+# We can get to passing mypy's strict mode by incrementally changing the
+# following global settings to true. This comes from mypy's documentation.
+
+warn_unused_configs = true


Right now they are all true - you mean you'd change all/some of these to false prior to adding this to CI?

Good catch! I was debating back and forth about this. If we keep all of these true, then we can ensure that new files will be consistent with mypy's strict configuration. Adding type hint annotations is much easier while writing code, so I'm thinking we should try out strict mode. If any of the settings cause any headaches, we can set them to be false.

In this case...I need to update the comment!

I agree. On the other hand, if you want this in the package I'm in favor of getting it into CI as soon as possible to at least stem the flow of new files without type hints. Maybe we can change some of these to false when adding this to CI to ease into it.

namurphy · 2023-12-29T20:56:36Z

@pheuer — thank you for the review! This is by no means the final version of the mypy configuration. We'll need to play around with it to see what it's like.

Looks good - shouldn't really change much for users/developers until added to CI, except for the few new minor added requirements.

True! I'm hoping to submit a PR to add mypy to CI soon, but we'll need to add some content to the contributor guide along with it.

One note - presumably between now and when CI is turned on, more changes will be made to the code and it's possible the list of exception files will change and need to be updated in the PR that turns on CI?

Yes, this is unfortunately true too. I'm planning to regenerate the configuration file unless I get it added to CI sooner rather than later. My biggest worry about this approach is that it'll be hard to maintain manually. But...this configuration is only a starting point.

Thank you again for the review — I appreciate it!

namurphy · 2023-12-29T21:10:46Z

plasmapy/diagnostics/charged_particle_radiography/detector_stacks.py

+        stopping_power: u.Quantity[u.J / u.m, u.J * u.m**2 / u.kg],
+        mass_density: Optional[u.Quantity[u.kg / u.m**3]] = None,


I missed these in #2421.

Add permissive mypy configuration

d4907fb

namurphy added the prototype 🏗️ Trying out an implementation on a trial basis label Dec 21, 2023

github-actions bot added the packaging Related to packaging or distribution label Dec 21, 2023

Add mypy to pre-commit

817140d

namurphy added 2 commits December 21, 2023 16:54

Remove mypy section in pyproject.toml

0e239d8

Add mypy.ini

affee6a

Setting up the mypy configuration in pyproject.toml was giving me a headache when I was trying out how to exclude files.

github-actions bot added CI Related to continuous integration linters For linters (e.g., flake8, ruff) and autoformatters (e.g., ruff, black, isort, Sourcery) labels Dec 21, 2023

Remove type syntax causing an error

4eb64b6

github-actions bot added the docs PlasmaPy Docs at http://docs.plasmapy.org label Dec 21, 2023

namurphy added 2 commits December 21, 2023 17:08

Skip directories when running mypy with pre-commit

5794d63

Be more specific about type: ignore

231a98c

Add changelog entry

87008ed

namurphy marked this pull request as ready for review December 21, 2023 22:46

namurphy requested review from rocco8773 and a team as code owners December 21, 2023 22:46

namurphy changed the title ~~Enable mypy with loose configuration~~ Enable mypy in pre-commit with initial configuration that ignores problematic files Dec 21, 2023

This was referenced Dec 22, 2023

Change unit annotations to Quantity annotations #2421

Merged

Improve type hint annotations in plasmapy.particles #2429

Closed

namurphy added 2 commits December 28, 2023 14:33

Merge branch 'main' of github.com:PlasmaPy/PlasmaPy into enable-mypy

5b7b406

Update mypy.ini following change to Quantity annotations

647df4b

github-actions bot added the plasmapy.analysis Related to the plasmapy.analysis subpackage label Dec 28, 2023

namurphy changed the title ~~Enable mypy in pre-commit with initial configuration that ignores problematic files~~ Enable mypy with initial configuration that ignores problematic files Dec 28, 2023

namurphy mentioned this pull request Dec 28, 2023

To what level should astropy be typed? astropy/astropy#15170

Open

Use pretty output for mypy

dccac87

namurphy added 2 commits December 28, 2023 17:26

Add type hints for empty __all__s'

4edda60

Update global settings for mypy

9cc0646

Update errors to ignore with newer version of mypy

9112260

namurphy commented Dec 29, 2023

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

Update 2424.trivial.rst

a42c8b3

namurphy commented Dec 29, 2023

View reviewed changes

namurphy added 4 commits December 28, 2023 20:57

Put back the backtick

b70cda4

Regenerate requirements file

6ae4501

Add error code to ignore

ea8740c

Merge branch 'enable-mypy' of github.com:namurphy/PlasmaPy into enabl…

9b81a53

…e-mypy

github-actions bot added the dependencies Related to updating dependency files label Dec 29, 2023

namurphy added 2 commits December 28, 2023 21:07

Add more per-file mypy error codes to ignore

badc705

Strangely, I was getting different results from mypy using a pre-commit hook vs running it locally. I think it's probably due to running different versio of mypy.

Add final missing error code

96122db

namurphy mentioned this pull request Dec 29, 2023

Add mypy configuration file that ignores existing errors on a per-file and per-error basis astropy/astropy#15794

Draft

1 task

pheuer approved these changes Dec 29, 2023

View reviewed changes

namurphy commented Dec 29, 2023

View reviewed changes

namurphy added 3 commits December 29, 2023 16:12

Update comment

1b5b780

Add comments about mypy type stubs packages

d263bbf

Remove mypy from pre-commit

a3bcf0c

namurphy enabled auto-merge (squash) December 29, 2023 21:17

Merge branch 'main' of github.com:PlasmaPy/PlasmaPy into enable-mypy

9295e10

namurphy changed the title ~~Enable mypy with initial configuration that ignores problematic files~~ Add mypy configuration file that ignores existing errors on a per-file and per-error basis Dec 29, 2023

namurphy merged commit 38b2361 into PlasmaPy:main Dec 29, 2023
14 checks passed

This was referenced Dec 29, 2023

Add a tox environment for running mypy #2431

Merged

Add mypy to continuous integration suite #2432

Merged

Get @particle_input to pass mypy's strict mode #2434

Closed

namurphy added the static type checking label Dec 30, 2023

This was referenced Jan 3, 2024

Add type stub file for @wrapt.decorator #2442

Merged

Switch from mypy to pyright for static type checking #2451

Closed

namurphy deleted the enable-mypy branch February 7, 2024 22:04

namurphy mentioned this pull request Mar 20, 2024

Decide on preferred set of mypy rules to adopt #2589

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mypy configuration file that ignores existing errors on a per-file and per-error basis #2424

Add mypy configuration file that ignores existing errors on a per-file and per-error basis #2424

namurphy commented Dec 21, 2023 •

edited

codecov bot commented Dec 21, 2023 •

edited

namurphy commented Dec 21, 2023

namurphy commented Dec 28, 2023

namurphy Dec 29, 2023 •

edited

pheuer Dec 29, 2023

namurphy Dec 29, 2023

pheuer left a comment

pheuer Dec 29, 2023

namurphy Dec 29, 2023

pheuer Dec 29, 2023

namurphy commented Dec 29, 2023

namurphy Dec 29, 2023

		stopping_power: u.Quantity[u.J / u.m, u.J * u.m**2 / u.kg],
		mass_density: Optional[u.Quantity[u.kg / u.m**3]] = None,

Add mypy configuration file that ignores existing errors on a per-file and per-error basis #2424

Add mypy configuration file that ignores existing errors on a per-file and per-error basis #2424

Conversation

namurphy commented Dec 21, 2023 • edited

Description

Motivation and context

Related issues

codecov bot commented Dec 21, 2023 • edited

Codecov Report

namurphy commented Dec 21, 2023

namurphy commented Dec 28, 2023

namurphy Dec 29, 2023 • edited

Choose a reason for hiding this comment

pheuer Dec 29, 2023

Choose a reason for hiding this comment

namurphy Dec 29, 2023

Choose a reason for hiding this comment

pheuer left a comment

Choose a reason for hiding this comment

pheuer Dec 29, 2023

Choose a reason for hiding this comment

namurphy Dec 29, 2023

Choose a reason for hiding this comment

pheuer Dec 29, 2023

Choose a reason for hiding this comment

namurphy commented Dec 29, 2023

namurphy Dec 29, 2023

Choose a reason for hiding this comment

namurphy commented Dec 21, 2023 •

edited

codecov bot commented Dec 21, 2023 •

edited

namurphy Dec 29, 2023 •

edited