ci: mypy config

[nstarman]

A redo of #12971 now that we have typing!

This PR provides a mypy configuration where all folders are ignored.
As a sub-package adds type hints the module may be removed from the top [[tool.mypy.overrides]] that ignores all errors and sub-package specific settings added in a new [[tool.mypy.overrides]], below.

e.g.

    [[tool.mypy.overrides]]
    module="*/astropy/cosmology/*"
    ignore_errors = false
    ...

Non-goals

1. type-hint anything in this PR.
2. force any sub-package to start adding type hints
3. decide where type hints will be stored: in the code, or as type stubs in a separate directory. This is resolved.

Goals

have a quality setup that:

1. allows any sub-package to start adding type hints.
2. works for either type hint location. This is resolved

With this PR, @namurphy's PRs for tox and CI can proceed.

[github-actions]

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

• Do the proposed changes actually accomplish desired goals?
• Do the proposed changes follow the Astropy coding guidelines?
• Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
• Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
• Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
• Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
• Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
• Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
• At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

[pllim]

Is the leading whitespace necessary? I don't see that in the other sections.

Suggested change

	python_version = "3.10"
	python_version = "3.10"

[nstarman]

hm. good point. I think @WilliamJamieson had a PR about improving our pyproject.toml layout... The leading space allows for section folding in IDEs.

[nstarman]

We definitely don't have any formal style for our pyproject.toml file. E.g.

[tool.coverage]

    [tool.coverage.run]

is different than our ruff config, where the subsection-that's-actually-a-section isn't indented.

[nstarman]

Not that I'm advocating for style anarchy. Actually it would be better to have a linter integrated with pre-commit that takes care of all these concerns for us...

[pllim]

Why does that sound so familiar? I am surprised it has not been done already by now.

[nstarman]

@namurphy, given PlasmaPy/PlasmaPy#1868, do you have any suggestions?

[namurphy]

I've had good experiences with pyproject-fmt, so much so that I forgot we'd added that hook! I'd suggest using a linter specific to pyproject.toml because checks against standard schemas for different configurations are likely to be either present already or added in the future, and these checks could potentially find configuration problems.

[nstarman]

Have you found a way to prevent pyproject-fmt from alphabetizing your toml? Or since the TOML is so rarely looked at, do you just let it alphabetize?

[namurphy]

Correction: it appears PlasmaPy doesn't use pyproject-fmt anymore. I just tried adding it back to PlasmaPy's pre-commit configuration, and in addition to the sorting, it also removed comment-only lines (tox-dev/pyproject-fmt#16). Since there are times when it really helps to have comment-only lines, we won't be able to use it until it's resolved...

[nstarman]

Thanks @namurphy for the info!

FYI for readers of this thread, I've resolved the whitespace inconsistencies manually. So this thread is resolved, even if the discussion for future improvements is ongoing!

[eerovaher]

units is a major bottleneck for type checking astropy, so I would rather not introduce any project level type checker configuration until units has been decently annotated. Adding annotations (anywhere, not just in units) to help with IDE tab-completion can be useful already, but that does not require configuring a type checker.

[eerovaher]

Very few maintainers have expressed any interest in adding type annotations and the number of people that have expressed interest in type checking astropy is even lower. It seems better to require sub-packages to explicitly opt in to type checking instead of setting files = ["astropy/**/*.py"] and then explicitly opting sub-packages out.

[namurphy]

A consideration here is whether we want subpackages that get added to Astropy in the future to be type checked with mypy by default. Because adding type hints is significantly easier when writing code for the first time, my inclination would be to set it so that new subpackages do get type checked by default. But, this consideration would only matter if any subpackages might be added in the next few years.

[pllim]

A consideration here is whether we want subpackages that get added to Astropy in the future to be type checked with mypy by default.

Unlikely we will have a new subpackage anytime soon (but never say never). When we do though, it is usually already an existing code/package. To require typing right off the bat would be a deal breaker. So 👎 from me.

[eerovaher]

Because adding type hints is significantly easier when writing code for the first time, my inclination would be to set it so that new subpackages do get type checked by default.

It is true that type checking is simpler if it is done from the very beginning, but we can't demand that from new sub-packages without an APE.

[nstarman]

Agreed that new sub-packages in principle new sub packages can be more easily typed, but in practice are unlikely to be given their origins from other sources.

Very few maintainers have expressed any interest in adding type annotations and the number of people that have expressed interest in type checking astropy is even lower. It seems better to require sub-packages to explicitly opt in to type checking ...

I think the picture is more complicated. It's true that few maintainers have wanted to add type annotations and fewer have started, but what's also true is that many users want it. (I have plenty of colleagues that have told me that they struggle to use Astropy for a few reasons and for many of them a significant one is because they aren't overly interested in programming and just want to get the job done and have justifiably gotten really used to the way that modern IDEs can surface information from type annotations and help do argument-by-argument completion of functions and classes. It just works. I know that I'm super used to it and am less-than-thrilled when I use a library that makes me dig around for the information. I'm looking at you jax.experimental.array_api with your lack of type annotations and docstrings). The bar for good, helpful user-facing code has been raised. So while it's true that few maintainers are doing anything, I think that reflects a failing on our part, not a reason to structure Astropy's config.

That being said, in its current form this PR is exactly equivalent to not opting in for any sub-package. And it will basically remain so for a while. The crucial difference between adding an opt-in list versus an opt-out list is that an opt-out list is a visible reminder of what remains to be done for Astropy to be more performant, have fewer bugs, and be more user friendly. I'm hoping that utils and cosmology and units will be some of the first libraries to remove themself from the opt-out list. I know users will appreciate it.

@eerovaher you are doing an excellent job adding type hints and other than my thanks right now, it's probably a mostly thankless task. Thank you for your work on utils.

[eerovaher]

We should indeed add type annotations that are helpful for IDE tab-completion, but that does not require configuring a type checker.

Ideally type checking would be helpful for developers, but in practice whenever I try to use mypy in astropy I immediately get overwhelmed by warnings from units and that is not helpful.

[nstarman]

We should indeed add type annotations that are helpful for IDE tab-completion, but that does not require configuring a type checker.

We do need to test the type annotations. RN the ones we are adding are completely untested. They look right...

[eerovaher]

The complexity and rigor of annotations that are required to satisfy type checkers is much higher than what can be useful for IDE tab-completion. I know that some of the annotations I've written are oversimplified so that a type checker will (rightfully) complain about them, but I still wrote them that way because having good return type annotations is helpful for IDEs even if some of the parameter annotations are sloppy. Furthermore, my expectation is that it will be simpler to start using a type checker when the fraction of annotated code is larger because then the checker should be able to point out more precisely what needs to be changed to make it happy.

I don't want us to end up in a situation where the few people that are adding annotations have to spend more time adding exceptions to the type checker configuration than they would spend on adding annotations themselves. And if early on our approach is to ignore (almost) every type violation then we can do that without writing anything down.

[neutrinoceros]

I feel like type hints too often take 100% of our focus in these discussions, when in fact they're not the only reason to use a type checker, so let me give a different perspective. Personally, the main reason I'd want to see astropy starting to use a type checker is that it'd enable using constructs from the typing module that are essentially declarations of intention which don't have any effect at runtime, like final, Final, override, overload, assert_never¹, Protocol or ReadOnly (new in Python 3.13). To me, these are the real game changers and they are the part the typing language that actually allows me to detect and fix defects through a type checker, but of course type hints play a big part in enabling this too. I'm not saying that type hints don't help finding bugs too (they do !), but in my experience they're higher cost, lower rewards in that area.

Footnotes

1. this one is not a no-op at runtime ↩

[nstarman]

@neutrinoceros, a good point! I use Protocol all the time, and TypedDict, and other useful runtime-features. override is also such a nice addition to typing.

[namurphy]

I very much like this PR! Thank you for doing it, and to everyone else for their feedback and work towards adding type hints! The approach taken here is a much better idea than the prototype I experimented with in #15794... 😮‍💨

The mypy docs contain a guide on using mypy with an existing codebase, which I've found to be incredibly helpful for PlasmaPy. This PR implements two of the recommendations from this guide:

• Make sure everyone runs mypy with the same options. ...
• Make sure everyone type checks the same set of files. ...

[The guide also recommends adding mypy to CI as soon as possible, which made the process of adding type hints to PlasmaPy significantly easier! But that's out-of-scope here and may be best discussed in https://github.com/astropy/astropy-APEs/pull/94, which I'm hoping to get back to one of these megaseconds... ⌚]

[nstarman]

@eerovaher, you wrote a good post about the levels of typing we want to do as a package.
I think package-wide consensus is now that we can add informative types to functions but we are not requiring the types to be perfectly correct. (I think that's your level 1). This is a great improvement over the previous state of affairs and I think it's well worth doing anywhere and everywhere.
Higher levels of typing relate to satisfying mypy (and distributing those types). I don't think us doing level 1 package-wide precludes doing level 2 in specific places, at the discretion of the package maintainer, e.g. to help Python optimize it's compilation. This PR establishes the architecture to permit level-2 (which requires some CI). If a package maintainer doesn't want to verify their type annotations, then that is the default state! It's functionally equivalent to an allow-list, but replaces the blanket ignore with package-specific ignores, which serves as a TODO log. We do this with .ruff.toml; IMO this is the best approach in this case too.

[eerovaher]

It's not clear to me who is supposed to maintain type checker configuration and how. Adding exceptions to every type checking error or warning doesn't sound very helpful, even if it is done on per-file and per-error basis. It is much simpler to ignore type checking by not running a type checker at all. An alternative that could be useful would be to try to address type checking errors as they are revealed, but no one has done that so far. Is anyone planning to start doing that? Maybe they should start by opening a pull request where they could post the type checker configuration that they used, and then we could evaluate if the improvements this enables are worth the effort.

I don't think us doing level 1 package-wide precludes doing level 2 in specific places, at the discretion of the package maintainer

A pull request that adds annotations to some set of functions that are useful for IDE tab-completion has no effect on the tab-completion of any other functions. This means that functions can be annotated independently from each other and project-level coordination is not required. Type checking is very different because adding annotations to one function could cause new type checking warnings in other functions, modules or even sub-packages, especially early on when the amount of annotated code is still very small. It seems to me type checking needs project-level commitment, or at least commitment from the key sub-packages such as units.

[...type checker configuration] serves as a TODO log. We do this with .ruff.toml

Addressing a Ruff rule violation does not cause brand new violations to appear in unedited code, but editing type annotations can cause brand new type checking violations in unedited code. So the ignores in ruff.toml can be treated as a TODO list where addressing any item removes it from the list permanently, whereas type checker configuration would be like a whack-a-mole game where errors can easily pop up again. They are quite different.

[neutrinoceros]

Is type-checking on the list of topics to be discussed at the Coordination Meeting yet ? it sounds more and more like it should be.

[pllim]

Does this count? astropy/astropy-project#349 (comment)

[neutrinoceros]

Thanks ! I missed it !

[astrofrog]

We should probably make sure that at least the big picture aspect of typing is discussed as a plenary just to make sure everyone is on the same page.