To what level should astropy be typed?

@nstarman

What is the question that needs to be resolved?

A bit of typing has entered astropy, with some sub-packages being substantially further along than others. We also have had a few bug reports pointing out that type information is missing. It may be good to decide package-wide how we want to approach this. Options:

Go with the flow, let people update as they please (which means an incomplete state for quite a while).
Embrace typing. If so, we need to think a bit on who is going to do the work. There seem to be two options:
- Add typing information to the modules proper. This likely means that subpackage maintainers will have to review quite a lot of PRs, even if they are not particularly interested in typing (or even disfavour it somewhat, because of the extra clutter).
- Add information in .pyi files (as numpy has done). This would mean only those who care about typing would do the work, but that only well set-up editing environments have this information readily available.

Personally, I have no particular preference, though I think I'd prefer to spend time reviewing bug fixes/enhancements (though I'd change my mind if there were, e.g., clear performance improvements to be had, as @nstarman suggested might be possible with mypy). So, I'd like a scheme where it is clear who is responsible for adding and maintaining the typing information (which may be easier with option 1!).

Describe the desired outcome

Some kind of decision on whether to add typing information as we go, and where to add it. If it is effectively option 1, then I think we are done, otherwise an APE may be needed.

I don't have time to attack this right now, but I think it would be awesome if astropy had type annotations. It's pretty annoying to me that I have to force mypy to ignore everything from astropy right now.

However, a possibly larger issue is that I don't think numpy has added support for generic ufuncs or array functions (correct me if I'm wrong), so if astropy.units did have typing, using any numpy function would ruin the type information.

Personally I'm most in favor of 2a (it's easiest to maintain), followed by 2b (at least we got it), then 1 (we got it where people care about it).

For some parts of Python typing is no longer optional, but required, e.g. dataclasses. For other parts it is best practice, e.g. ClassVar to differentiate between class variables and instance variables. And last, by working with numpydoc we can embrace something that sphinx-napoleon enables, which is using type information in the docstring Parameter, Returns, etc sections which would be very nice for complicated types.

def func(x: str | bytes | os.PathLike | ReadableFileLike | tuple[str, str] | ... | Sequence[str | bytes os.PathLike | ReadableFileLike] | ...):
    """Description.

    Parameters
    ——————-
    x  # (no longer need to explain this, the types are inserted at docs compilation)
        But a text explanation is nice.
        A path to a file or the file itself. Can be a sequence, in which case the operation is performed for
        each element in the sequence. A 2-elt tuple indicates something special.
    """

Furthermore, yes, well-typed code can be better compiled by Python than untyped code. Compilation tools like mypyc can provide further speed boosts.

Very good points above! And thank you for raising this issue.

I'd be hesitant to have typing information in .pyi files. When the typing is in a separate file, it is harder to maintain, which makes it less likely it will be maintained. With that said, there may be some situations where the tradeoffs are different and using a .pyi file may be the better choice. So, I really like option 2a.

One approach we've taken in PlasmaPy is to create typing constructs for commonly used situations. For example, we have ParticleLike which is an alias of str | Integral | Particle | CustomParticle | Quantity with its own docstring. There could be a typing alias like QuantityLike for astropy.units too.

I've also been meaning to look into tools that could automatically add type hints.

@mhvk

I'm not answering exactly the question @mhvk asked, but I think this will still be relevant for the discussion.

The way I see it there are three tiers to typing. In the order of increasing strictness and complexity they are:

having type annotations in public functions so that IDEs can provide helpful autocompletion
having type annotations that mypy is happy with
having type annotations that mypy strict mode is happy with.

Tier 1. should be uncontroversial because astropy public API already contains type information in docstrings. We would just be writing down the same information in a new format that IDEs can understand. There might be tools that could do this conversion automatically. I don't think doing this requires any policy decisions at the project level.

Once a large enough fraction of astropy is covered with type annotations that are useful (for autocompletion), then we can consider discussing opting into using mypy. It should be easier then because mypy becomes more capable and useful as the fraction of annotated code increases. That means mypy itself can tell us what changes are needed to make it happy. But I don't think we need to discuss using mypy any time soon.

@namurphy mentioned introducing definitions for commonly used union types. astropy already defines such types. We would just have to rewrite these definitions in a way IDEs and type checkers can understand.

@mhvk

I agree with this on the whole, with the caveat that we currently compile performance-necessary code. With mypy+mypyc applied to very select and agreed-upon sections of the library we can increase performance of some currently-not-compiled-but-performance-desired code. That's just saying we might want to have some tier-3 stuff right away. NOT EVERYWHERE, just where we want. E.g. @mhvk and I are interested in this for some units stuff. This naturally fits in with our other compiled C code, so I think it falls under the same set of guidelines, we just need to add the requisite settings to our config.

But I think the important takeaway is that this is a related, but separate, discussion about how to start with tier-1 and shouldn't stop us from helping our users and improving the code.

I'm strongly in favor of having type annotations in the modules rather than .pyi stub files. Seeing the annotations directly is very helpful when viewing the code in any editor and especially on GitHub.

I think that "Go with the flow" and "Embrace typing" are not mutually exclusive, and I am taking this incremental approach with the dozens of packages we use in Chandra operations. Many of these are 10+ years old and have varying degrees of rigor. So my process is:

Include type annotations in most new code
Update existing code based on available time and motivation.
Don't ever bother with getting to the point of having a static typing tool like mypy pass. In my experience and for our purposes this is just too much work. Of course astropy (as a core community library) is a bit different.

I find that any level of type hints is helpful and that it can be entirely incremental. It can even be little things like taking 15 minutes to add annotations to a few functions.

For astropy I think a big issue is dealing with the very flexible API of some of the most prominent classes. Basically the original design approach for some core classes like SkyCoord or Table or Time was to "just work" for a wide variety of inputs. So for example Time can be initialized with essentially anything since the user can register a custom time format that parses that "anything".

In these cases we could define type aliases like TimeLike and TableLike that correspond to the glossary definitions. They would make no attempt to be comprehensive but just give a clear indication of an argument that is going to be coerced to a Time or Table like time = Time(time) in the function.

It would likely be very beneficial for an incremental process to provide a few things:

A typing module with core types.
A typing documentation page ala NumPy with some descriptions as well as developer documentation on best practices for adding type annotations to astropy.
A new typing label for astropy PR's and hopefully some effort by the experts in this thread to do PR review.

I think that an incremental crowd-sourced approach can be effective since these PR's are in the good-first-issue category of requiring very little detailed astropy knowledge.

Related note, typing syntax is about to get a whole lot cleaner — https://peps.python.org/pep-0695.
And with our ruff-based tooling we should be able to auto-upgrade the syntax when we hit py3.12+ (e.g. https://docs.astral.sh/ruff/rules/non-pep695-type-alias/).
All's that to say we should start typing now and let python + linting clean it up later.

@taldcroft

It sounds like a bit of a convergence on option 1, of just adding typing as we go, and clearly the preference is for doing it inside the .py files (i.e., not adding .pyi). I'm certainly happy with that. I do agree with @taldcroft about the first good steps, of having a typing model with definitions and associated documentation.

So, maybe what needs to be done now is to open an issue about creating a typing module? (and close this one)

p.s. To slightly argue for a less as-we-go mode: most of our docstrings are, I think, fairly good & standard in laying out the type of the parameters - it should be possible to have some kind of script that transfer that information to the signature. Indeed, maybe that exists already?

So, maybe what needs to be done now is to open an issue about creating a typing module?

Agreed. I also think most modules will end up having a domain-specific set of types and thus their own typing module (though it may be private). For many things we can organically discover what are the common types and move those to the common typing module.

@taldcroft

I'm joining the emerging consensus on option 1 and I think that the roadmap drawn by @taldcroft makes perfect sense.
I have 2 minor points to add

to avoid painting ourself into a corner, maybe we shouldn't have a public typing module from the get go; since change is to be performed in increments, maybe we don't want to commit forever to every early decision.
quoting from To what level should astropy be typed? #15170 (comment) (@eerovaher)

Once a large enough fraction of astropy is covered with type annotations that are useful (for autocompletion), then we can consider discussing opting into using mypy. It should be easier then because mypy becomes more capable and useful as the fraction of annotated code increases. That means mypy itself can tell us what changes are needed to make it happy. But I don't think we need to discuss using mypy any time soon.

I would agree here on the condition that we don't add a py.typed marker file until we've started validating with mypy (or another type checker for that matter). I don't know that it's been discussed already but this is the marker file that type checker use to infer wether or not they should bother parsing type information from our source code. This is important when type-checking downstream, and will be necessary to resolve issues like the one @byrdie hinted to in #15170 (comment)

A .pyi stub file may be necessary for dynamically generated content. I'm thinking especially for the units in astropy.units. I'm wondering if it would be possible to dynamically generate this stub file, since it would go out of date when metric prefixes are added, for example.

@namurphy

@namurphy - since we generate the docstring for the various submodules with unit definitions automatically, it should be possible to generate the .pyi files too. The main difference is that we do not want to run this on every import, but just on installation. Relevant code:

astropy/astropy/units/utils.py

Line 80 in 6568d90

def generate_unit_summary(namespace):

Would it make sense to start by enabling mypy in pre-commit, while initially setting the mypy configuration to ignore all existing errors?

I've been attempting to do that for PlasmaPy (https://github.com/PlasmaPy/PlasmaPy/pull/2424)...and got mypy to pass by temporarily ignoring all errors in only 79 files! 🥲 Our next step will be to gradually improve type hint annotations throughout the package and hopefully get mypy to eventually pass in strict mode. I'm planning to look in the broader pythoniverse for which tools are best for automagically add type hint annotations...since without the tools, it'll probably take a lot of effort. I'd be happy to share my experiences about what does or does not work well.

About a document for adding type hint annotations, I might suggest a wiki page rather than APE.

Perhaps we could add a page to Astropy's contributor guide? I'm planning to add a section or page to PlasmaPy's contributor guide on type check annotations and static type checking, which could potentially be adapted to Astropy's as well.

The main reason why I was suggesting an APE was that I figured that this would be a big enough change that we'd need the coordinating committee to decide. But, if we could get by without an APE, that would be fantastic!

I also wanted to say more about why I'm suggesting that a static type checker be added to Astropy's CI as the first step before moving towards any of the tiers of typing. 🤔

We have been adding annotations to functions in PlasmaPy during most of its development. We used type hints when possible, but had unit annotations in a lot of our functions (i.e., def f(x: u.m): ...). We recently expanded one of our decorators to work with type hint annotations of the style def f(x: u.Quantity[u.m]): ....

Long story short, we ended up with a code base that was ∼half annotated, but without a consistent way to check that the annotations are actually correct. Since starting to use mypy in the last few weeks 🥳, I've frequently had to correct pre-existing annotations after mypy found errors in them. If type hint annotations start being added to Astropy without having a consistent way to check that the annotations are correct, then it will probably be necessary to revisit those annotations again after static type checking has been enabled in CI.

But that doesn't mean that the static type checker should be enabled globally. Rather it should be enabled gradually. I really like #12971 as a first step because it configured mypy to not check any files. We could have mypy ignore files until we start adding type hint annotations to them, at which point we could enable mypy for those files and use it to check that the type hints we add are actually correct. That way, it's less likely that we'd have to revisit those files later.

https://cosmology.readthedocs.io/projects/api/en/latest/introduction.html for an introduction to typing that focuses on structural sub-typing.

@namurphy

If type hint annotations start being added to Astropy without having a consistent way to check that the annotations are correct, then it will probably be necessary to revisit those annotations again after static type checking has been enabled in CI.

@namurphy - can you clarify what it means to check that the annotations are "correct"? That seems like a question that can have many different answers. I have no experience with static type checking so I need some help here, but maybe you mean "passes mypy" with some level of checking enabled.

This is actually an important point to establish in this discussion of "to what level should astropy by typed". Are we going to require that new type annotations pass some static type check analysis prior to being committed? I suspect this would be a show-stopper for an effort to incrementally add type annotations to astropy.

Requiring an entire file to be "correct" means that adding type annotations cannot be done on a few functions at a time. Knowing that getting all 4200 lines of astropy.table.table to pass is going to be a monumental project, I'm not likely to even start. Instead I thought the idea was to provide some guidance¹ and then solicit volunteers to just start adding type annotations at a level of even one function at a time. Quality control would initially be in the form of experienced human reviewers checking the annotations.

One example is how and where to define types like TableLike which includes a big mess of possibilities for an object that can initialize a Table. This is a case where I think the type has value for documentation but is unlikely to be helpful in any static code analysis to actually prevent a bug. ↩

About the APE vs. contributor guide -- if we are talking about a new policy that adding type annotations goes hand-in-hand with enforcing some level of static type checking, that would definitely require an APE (which would certainly be controversial).

Just starting to add annotations can be done any time at the discretion of package maintainers.

Although I agree that there exist tools that can be helpful for adding type annotations, I think at this point it might be best if any interested developers run such tools locally. This can lead to different developers using different tools and settings, but if we ever get to a point where we might want to add a type checking CI job then it could be for the better because then we could have a more informed discussion about which tools and settings the CI job should use.

@nstarman

@nstarman — that looks like an awesome resource!

@taldcroft and @eerovaher — thank you for continuing to bring up important points!

@namurphy - can you clarify what it means to check that the annotations are "correct"? That seems like a question that can have many different answers. I have no experience with static type checking so I need some help here, but maybe you mean "passes mypy" with some level of checking enabled.

Operationally, that's pretty much exactly what I mean. It's worth taking a look at the mypy error codes enabled by default and the error codes for optional checks.

Are we going to require that new type annotations pass some static type check analysis prior to being committed?

The approach I've been taking in PlasmaPy has been to:

Set the mypy configuration file to ignore all existing errors on a per-file and per-error basis (Add mypy configuration file that ignores existing errors on a per-file and per-error basis #15794 and Add mypy configuration file that ignores existing errors on a per-file and per-error basis PlasmaPy/PlasmaPy#2424). PRs are now required not to add any new categories of mypy errors per file. But, if a file already had return-arg errors, then mypy would ignore any new return-arg errors that get added.
Go through individual files and remove one error code at a time, and get mypy to pass (e.g., Improve type hint annotations for @particle_input PlasmaPy/PlasmaPy#2443).

For Astropy, I'd suggest enabling mypy only for a single subpackage or module at first to make the process even more gradual.

I've also been using tools like autotyping to automagically add type hints. The --safe flags were pretty reliable, while the --aggressive flags were a good first guess and required some iteration.

Hi! Would someone kindly give me a brief rundown of what an APE is? I looked on google, but I didn't get definitive answers.

Astropy Proposal for Enhancement (https://github.com/astropy/astropy-APEs)

OK. We should write an APE regardless, as a Plan B. Bc if we go the wiki route, there is a non-zero chance that someone will want an APE.

See https://lwn.net/Articles/958326/ "Growing pains for typing in Python" for a summary of the discussion on typing within python itself. Let's ensure that /the focus should be on user experience and not "making programming harder for humans in order to make it easier for IDEs"/.

Thank you for posting this article!

See https://lwn.net/Articles/958326/ "Growing pains for typing in Python"

Looks like that article is behind a paywall at the moment, but will become freely available on Jan 25.

Let's ensure that /the focus should be on user experience and not "making programming harder for humans in order to make it easier for IDEs"/.

A flip side of this is that making it easier for IDEs can turn into making things easier for humans. There have been a few times recently where type hints helped me find errors as I was writing code. For example, PyCharm highlighted when I was calling a method that only existed on some of the types in the type hints, so I was able to fix it in the moment without having to run tests or mypy.

With all that said, my biggest worry about type hints is adding a barrier to entry to new contributors, so I'm particularly curious about what the article says.

I just started a PR for an APE on the process for gradually enabling static type checking to Astropy. If anyone wants to work on this too, please let me know!

It's still in the early stages, and I want to make sure that the APE encompasses the important concerns and considerations brought up in this incredibly helpful discussion.

@taldcroft

Not sure whether discussion should now move to the APE, but it is always great to see actual examples, and while looking at #15914 (which introduces typing in units.physical), there were a few things that stood out to me:

Sometimes useless type information is added, like __eq__(self, other: object) -> bool - this particular example may well be wrong (see discussion at https://github.com/astropy/astropy/pull/15914/files#r1462012376), but I really want to ensure we agree now that we are not going to add extra characters that carry zero information to a developer trying to edit things just to please a type checker. If a type checker cannot handle the obvious, the expectation should be that an issue is raised with the type checker, not that we adjust our code.
Some classes have flexible inputs/outputs. In https://github.com/astropy/astropy/pull/15914/files#r1460623166, @overload was mentioned (again, the particular example may not be relevant), which made me think of stuff I've seen in numpy, like this small part of the typing of np.dtype

    # Builtin types
    @overload
    def __new__(cls, dtype: type[builtins.bool], align: builtins.bool = ..., copy: builtins.bool = ..., metadata: dict[builtins.str, Any] = ...) -> dtype[np.bool]: ...
    @overload
    def __new__(cls, dtype: type[int], align: builtins.bool = ..., copy: builtins.bool = ..., metadata: dict[builtins.str, Any] = ...) -> dtype[int_]: ...
    @overload
    def __new__(cls, dtype: None | type[float], align: builtins.bool = ..., copy: builtins.bool = ..., metadata: dict[builtins.str, Any] = ...) -> dtype[float64]: ...
    @overload
    def __new__(cls, dtype: type[complex], align: builtins.bool = ..., copy: builtins.bool = ..., metadata: dict[builtins.str, Any] = ...) -> dtype[complex128]: ...
    @overload
    def __new__(cls, dtype: type[builtins.str], align: builtins.bool = ..., copy: builtins.bool = ..., metadata: dict[builtins.str, Any] = ...) -> dtype[str_]: ...
    @overload
    def __new__(cls, dtype: type[bytes], align: builtins.bool = ..., copy: builtins.bool = ..., metadata: dict[builtins.str, Any] = ...) -> dtype[bytes_]: ...

I'd strongly suggest that we agree in advance that this kind of stuff does not belong with the regular code, but has to be delegated to a .pyi file.

More generally, I think this reinforces @taldcroft's suggestion to start with adding type information to public functions, and not worry about getting mypy to pass, etc. Indeed, I think it might be wise not to put any checkers in CI for quite a while, to avoid getting drawn in to over-typing -- some of the ruff rule stuff is getting me grumpy enough as it is!

@taldcroft

More generally, I think this reinforces @taldcroft's suggestion to start with adding type information to public functions, and not worry about getting mypy to pass, etc. Indeed, I think it might be wise not to put any checkers in CI for quite a while, to avoid getting drawn in to over-typing -- some of the ruff rule stuff is getting me grumpy enough as it is!

Actually, that bit about the CI is an implementation detail, which is probably best discussed with the APE. But the more general point is that I really want to avoid the tools dictating us.

Navigation Menu

Repository navigation

To what level should astropy be typed? #15170

What is the question that needs to be resolved?

Describe the desired outcome

17 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

To what level should astropy be typed? #15170

Description

What is the question that needs to be resolved?

Describe the desired outcome

Activity

roytsmart commented on Aug 14, 2023

nstarman commented on Aug 14, 2023

namurphy commented on Aug 18, 2023

eerovaher commented on Dec 6, 2023

nstarman commented on Dec 6, 2023

taldcroft commented on Dec 10, 2023

nstarman commented on Dec 10, 2023

mhvk commented on Dec 10, 2023

nstarman commented on Dec 10, 2023

neutrinoceros commented on Dec 11, 2023

namurphy commented on Dec 27, 2023

mhvk commented on Dec 27, 2023

namurphy commented on Dec 28, 2023

17 remaining items

namurphy commented on Jan 10, 2024

namurphy commented on Jan 10, 2024

nstarman commented on Jan 10, 2024

taldcroft commented on Jan 10, 2024

Footnotes

taldcroft commented on Jan 10, 2024

eerovaher commented on Jan 10, 2024

namurphy commented on Jan 10, 2024

tactipus commented on Jan 15, 2024

nstarman commented on Jan 15, 2024

tactipus commented on Jan 15, 2024

mhvk commented on Jan 17, 2024

namurphy commented on Jan 17, 2024

namurphy commented on Jan 19, 2024

mhvk commented on Jan 24, 2024

mhvk commented on Jan 24, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions