Add mypy configuration file that ignores existing errors on a per-file and per-error basis by namurphy · Pull Request #15794 · astropy/astropy

namurphy · 2023-12-29T20:24:04Z

Description

This prototype PR adds a mypy configuration file, mypy.ini, that ignores existing errors on a per-file and per-error basis. When I run mypy . in the top-level directory of this repo with this file, it passes using mypy v1.8.0! 🎉 🎂 🥳 🎈 🎆 🥦

The configuration file includes global settings near the top that can gradually be set to true in order to globally approach mypy's strict mode.

This PR does not add mypy to the suite of continuous integration tests, since doing so may require an Astropy Proposal for Enhancement (see astropy/astropy-APEs#92).

Motivation

Currently Astropy has no means to check that type hint annotations are correct, but there has been discussion over the last year or two about doing so, especially in #15170. If we want to methodically add accurate type hint annotations to Astropy, two steps that we'll need to take are:

Get a static type checker like mypy to pass on the repo, such as by ignoring existing errors
Add the static type checker to the suite of CI tests

This PR is a possible path forward for the first step, but there are other alternatives.

Backstory

This PR came about because I wrote a short script for PlasmaPy/PlasmaPy#2424 that parses the output of mypy and creates the sections in mypy.ini to ignore existing errors on a per-file and per-error basis. Since it was pretty straightforward to run for Astropy too, I decided to submit a demo/prototype PR.

Consequences

This PR would give Astropy a mypy configuration that passes, which would enable contributors to make progress on typing certain portions of Astropy.
If mypy is added to CI after merging this PR, then we could ensure that each existing module would not get any new categories of typing errors. Similarly, we could ensure that new files would not have any unintended typing errors.
The fine-grained approach would let maintainers go file-by-file and error-by-error to improve typing in Astropy.
The sections specifying the per-file and per-error ignores were created mostly automatically which a script that I plan to share. It would require extra care to mix manual configuration settings on a per-subpackage or per-module basis.
The per-file configurations would be annoying to manage manually due to its size. Since mypy is still changing over time, new errors could crop up that would need to be added.

Related issues

Closes #12970. This is similar to #12971, but this PR takes a much more fine-grained approach.

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2023-12-29T20:24:30Z

namurphy · 2023-12-29T20:24:42Z

+# typing errors should not be reported for specific modules. As we
+# gradually make Astropy consistent with mypy's strict mode, we can
+# remove the disabled error codes file-by-file.
+


Almost everything below here was created automatically.

It would be great to add separation comments between the modules

e.g.

[mypy-astropy.conftest] disable_error_code = arg-type,import-not-found ##################################################################### [mypy-astropy.constants.codata2010] disable_error_code = var-annotated

Should be pretty straightforward to do that. Glad we could hash this out! 🙃

github-actions · 2023-12-29T20:25:07Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

namurphy · 2023-12-29T20:28:25Z

+enable_error_code = ignore-without-code
+
+# 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
+warn_redundant_casts = true
+warn_unused_ignores = true
+strict_equality = true
+check_untyped_defs = false
+disallow_subclassing_any = false
+disallow_untyped_decorators = false
+disallow_any_generics = false
+disallow_untyped_calls = false
+disallow_incomplete_defs = false
+disallow_untyped_defs = false
+no_implicit_reexport = false
+warn_return_any = false
+extra_checks = false


These settings come from [mypy's doc page on introducing enabling mypy on an existing code base]. The doc page on mypy configuration files describes the individual settings.

My one larger suggestion is to have the configurations set on a per-subpackage basis rather than globally for all of Astropy.

@nstarman — just to make sure I understand, did you mean having a block of settings like this so that configuration variables like warn_redundant_casts, strict_equality, check_untyped_defs, etc. could be defined on a per-subpackage basis? I like that idea, and I think I'll probably apply that to PlasmaPy as well.

Alternatively, did you mean to have the per-file ignores become per-subpackage ignores like in #12971? That would also be a valid approach. The ignores would become less fine-grained, but mypy.ini would become shorter and probably more maintainable.

Thank you!

The former 👍. I just imagine that some subpackages will achieve strict=True well before others.

nstarman

This is looking great. I'm happy with this type of incremental configuration.
My one larger suggestion is to have the configurations set on a per-subpackage basis rather than globally for all of Astropy. We will hopefully get to strict=True globally, but this is good for incremental typing.

nstarman · 2023-12-30T21:28:23Z

+# typing errors should not be reported for specific modules. As we
+# gradually make Astropy consistent with mypy's strict mode, we can
+# remove the disabled error codes file-by-file.
+


It would be great to add separation comments between the modules

e.g.

[mypy-astropy.conftest] disable_error_code = arg-type,import-not-found ##################################################################### [mypy-astropy.constants.codata2010] disable_error_code = var-annotated

mhvk · 2023-12-31T18:23:33Z

I think this approach makes a lot of sense. Looking forward, is there an option to detect automatically that a file actually does not have errors any more? A bit like an pytest.mark.xfail will give an error if the test actually passes? If not, I guess the idea would be to, every once in a while, just regenerate the file?

namurphy · 2024-01-02T18:36:50Z

I think this approach makes a lot of sense. Looking forward, is there an option to detect automatically that a file actually does not have errors any more? A bit like an pytest.mark.xfail will give an error if the test actually passes? If not, I guess the idea would be to, every once in a while, just regenerate the file?

Thank you for the feedback!

I don't know of a way to automatically check that a file actually does not have any errors anymore. In the short to intermediate term, it would probably the most sense to regenerate the file quasiperiodically. If we wanted to be fancy, we could create a GitHub Action as a cron job to create a PR that regenerates this file. I don't know if it would be worth the effort. The regeneration would probably be necessary because it seems like adding types to one fine can sometimes resolves errors in another file.

The mypy.ini from this PR is probably best thought of as temporary. After enough progress has been made on typing, we could switch to having # type: ignore[error-code] comments on each individual line that an error occurs in. In that case, it would be possible to set mypy to generate an error if there is a # type: ignore comment that is no longer being used. There probably exist tools that automatically add the # type: ignore comments too, which would also be a valid approach. I'm hesitant to do that yet because that would probably change nearly every .py file in Astropy, and I also kind of like having mypy.ini as a checklist.

pllim · 2024-01-03T22:55:28Z

I don't think this needs a change log. We don't do change log for black or ruff fixes.

nstarman · 2024-01-04T14:21:54Z

+[mypy-astropy.coordinates.spectral_quantity]
+disable_error_code = assignment,attr-defined
+
+[mypy-astropy.coordinates.tests.accuracy.generate_ref_ast]


Do we want to type check the test files? I generally don't in my own code.

me neither, but I think it's a good idea nevertheless: test code is a good source of user code, checking it might reveal that our type annotations are too strict compared to what we actually support and test

Alternatively for the tests we could turn on beartype and do run-time checking of Astropy, checking all the type annotations without having to add type hints to the test suite.

from beartype.claw import beartype_this_package # <-- hype comes beartype_this_package()

I don't know that we need to add type annotations to tests but would definitely be good to have a way to have the tests actually test the annotations in the main code optionally.

The bear-type route in the tests will accomplish this (testing the type hints without having to type the tests) and should only add a few lines to the test code + a test dependency. This also has the added benefit of testing runtime type checking! Which some users will definitely want.

Interesting to hear about beartype!

For PlasmaPy, my current thinking is to include type hint annotations for tests since they can sometimes really help with test readability and maintainability...especially for when we're using @pytest.mark.parametrize or are somewhat complicated. I would be in favor of including type hint annotations for tests as well, but probably at a lower priority than for the application code.

Adding type annotations to tests can be worthwhile if it helps IDEs offer better autocompletion.

namurphy · 2024-01-08T20:56:27Z

A follow-up to this exploratory PR: I mentioned in this post that it's worth considering alternatives to mypy such as pyright.

nstarman · 2024-01-10T19:13:30Z

+          examples
+          )
+
+enable_error_code = ignore-without-code


Suggested change

enable_error_code = ignore-without-code

enable_error_code = ignore-without-code

plugins = numpy.typing.mypy_plugin

I think we will use this plugin frequently.

https://pydoc.dev/numpy/latest/numpy.typing.mypy_plugin.html

Making a note to add this to PlasmaPy!

namurphy · 2024-01-10T21:05:02Z

Some follow-up thoughts about this PR after the discussion in #15170 and continuing to add type hints to PlasmaPy:

This particular PR would enable mypy across the board for Astropy, even while ignoring all existing errors. I'm currently thinking that the first step should be more like what is proposed by @nstarman in Opt-in mypy settings #12970 & mypy incremental adoption settings #12971 where mypy is initially configured to ignore errors in all subpackages.
mypy could then be enabled on a single module or subpackage using a configuration that ignores all existing errors. The configuration could be something like in this PR, except more limited in scope.

This would make the rollout of static type checking more gradual, which is key.

github-actions · 2024-06-01T05:42:31Z

Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.

If you believe I commented on this pull request incorrectly, please report this here.

namurphy · 2024-06-04T15:38:26Z

This PR is superseded by #16312, which I think is a much better approach.

Add initial mypy.ini file

706212e

namurphy commented Dec 29, 2023

View reviewed changes

Add changelog entry

ce45dac

namurphy mentioned this pull request Dec 29, 2023

To what level should astropy be typed? #15170

Open

nstarman reviewed Dec 30, 2023

View reviewed changes

Update to Python 3.10+ in mypy.ini

1d68901

namurphy mentioned this pull request Jan 2, 2024

Change global mypy configurations to per-subpackage configurations PlasmaPy/PlasmaPy#2438

Open

pllim added this to the v6.1.0 milestone Jan 3, 2024

pllim added no-changelog-entry-needed dev-automation labels Jan 3, 2024

pllim reviewed Jan 3, 2024

View reviewed changes

nstarman reviewed Jan 4, 2024

View reviewed changes

This was referenced Jan 4, 2024

Coordination Meeting 2024 astropy/astropy-project#349

Closed

Add a tox environment to run mypy for static type checking #15815

Closed

nstarman reviewed Jan 10, 2024

View reviewed changes

nstarman mentioned this pull request Jan 10, 2024

Add pandas-stubs to typing dependencies #15849

Merged

aarchiba mentioned this pull request Feb 25, 2024

mypy checking as part of CI nanograv/PINT#1725

Open

astrofrog modified the milestones: v6.1.0, v7.0.0 Apr 4, 2024

namurphy mentioned this pull request Apr 19, 2024

ci: mypy config #16312

Closed

mhvk added the typing related to type annotations label May 30, 2024

github-actions Bot added the Close? Tell stale bot that this issue/PR is stale label Jun 1, 2024

namurphy closed this Jun 1, 2024

namurphy deleted the mypy-configuration-file branch June 1, 2024 13:55

eerovaher mentioned this pull request Dec 10, 2025

Add basic mypy configuration #19044

Merged

1 task done

namurphy mentioned this pull request Jan 28, 2026

APE: An initial roadmap for static type checking astropy/astropy-APEs#94

Open

Conversation

namurphy commented Dec 29, 2023

Description

Motivation

Backstory

Consequences

Related issues

Uh oh!

github-actions Bot commented Dec 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Dec 29, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nstarman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhvk commented Dec 31, 2023

Uh oh!

namurphy commented Jan 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astrofrog Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

namurphy commented Jan 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

namurphy commented Jan 10, 2024

Uh oh!

github-actions Bot commented Jun 1, 2024

Uh oh!

namurphy commented Jun 4, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

astrofrog Jan 4, 2024 •

edited

Loading