APE 25ish: An initial roadmap for static type checking #94
Conversation
There was a problem hiding this comment.
Thank you for starting this ! I feel that two important points may be missing at the moment:
- we ought to acknowledge and emphasis the expected gains (and costs ?) of having type-hints, and of static checking, from a downstream developers/maintainers perspective
- some tools (including mypy, last time I checked) will not attempt to discover type hints in a package unless it contains a
py.typedmarker file. This means that as long as we don't include it anywhere, our type annotations, while not statically checked, are also invisible from the perspective of someone checking downstream code. I think we'll have to include it to make our effort visible and useful, and that running a type checker ourselves is a necessary step that needs to happen before it. Meanwhile, as long as we don't include it, we can treat type hints as private details, which I assume will help with early implementation.
| 1. Requiring type hint annotations that mypy doesn't complain about | ||
| adds an extra step to contributing to Astropy... | ||
|
|
||
| 1. An additional step is required to contribute to Astropy. |
There was a problem hiding this comment.
Yep! This is still an early draft so I was trying out different wordings and ways to organize the doc.
|
|
||
| 1. An additional step is required to contribute to Astropy. | ||
|
|
||
| 1. Type hint annotations may add a barrier to entry for contributing |
There was a problem hiding this comment.
This also feels similar to the first two points, but I guess maybe you mean to say that existing type hints may make the code harder to read from a newcomer's perspective ? This is something I've heard in similar contexts, coming from seasoned devs, and I'm honestly not convinced it's a valid point, but it's nonetheless an opinion that exists.
| #. Helps introspection and tab completion in Jupyter notebooks | ||
| #. Helps IDEs to make things easier for humans | ||
| #. Helpful for downstream package maintainers |
There was a problem hiding this comment.
These are benefits of adding type annotations, and I enthusiastically agree with these benefits. Static type checking is not required to see these benefits for humans.
| #. Can find programming errors that would be difficult to find | ||
| otherwise |
There was a problem hiding this comment.
It would be useful to know what level of mypy checking is required to realize this benefit, along with an example of a module that is typed to that level.
One of my initial bad experiences with enabling type checking was using VS Code pylance/pyright on a small module at the "basic" setting. After several hours trying and failing to get to zero errors I gave up. Maybe mypy is better and the technology has advanced.
|
|
||
| 1. **Do not enable static type checking of Astropy.** Type hint | ||
| annotations have begun to be added to Astropy. At present, there is | ||
| no way to check the validity or accuracy of type hint annotations. |
There was a problem hiding this comment.
Some of the comments and type hints in astropy/astropy#15914 seem to highlight that checking validity of type annotations is sometimes not possible or useful. I'm thinking of object or UnitLike or PhysicalTypeLike annotations. Basically we have some protocol that is documented in a long paragraph and implemented in code, but is difficult/impossible to express as a type annotation.
This is certainly common in my subpackages table, time, io.ascii. To be sure there are many places where the types are reasonably narrow, but often they are not.
namurphy
changed the title
|
It's going to be a while before I can get back to this PR, so if anyone else wants to work on this, please let me know! My main goal here has been to encapsulate the advice given in the mypy docs on enabling static type checking on an existing code base. |
This PR will add an APE that proposes a path forward for enabling static type checking in Astropy's continuous integration suite. If anyone is interested in being a co-author, please let me know!
The process would be more or less to:
astropy.cosmologyandastropy.units?)It's currently in an early draft state, and might be best though of as being in the brainstorming stage 🧠 🌩️. Big picture feedback would be especially appreciated! In particular, we will need to address the really valid concerns and considerations that have been brought up in astropy/astropy#15170.
I'm also hoping to avoid being over-prescriptive, so as to give us flexibility and avoid the need to amend this APE when ruff implements a full static type checker. 🙃
Closes #92.