To what level should astropy be typed?

[mhvk]

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:

1. Go with the flow, let people update as they please (which means an incomplete state for quite a while).
2. 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.

[byrdie]

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.

[nstarman]

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.

[namurphy]

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.

[eerovaher]

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:

1. having type annotations in public functions so that IDEs can provide helpful autocompletion
2. having type annotations that mypy is happy with
3. 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.

[nstarman]

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.

[taldcroft]

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.

[nstarman]

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.

[mhvk]

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?

[nstarman]

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.

[neutrinoceros]

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

1. 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.

2. 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)

[namurphy]

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.

[mhvk]

@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):

[namurphy]

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.

[mhvk]

@namurphy - I'm not sure it is not a good idea to start requiring typing in new PRs; I think this should be opt-in by submodule maintainers who have done the basics in their modules and are willing to do the reviewing. I don't think we should make contributing harder before we have the basics in place, and have documented where to find the existing astropy types, etc. That said, once we have that, I'm all for letting mypy check PRs to submodules that are (mostly) typed.

Or might it be possible to do it on a per-file basis? If it passes mypy on main, it should pass on the PR? Or is this what you meant? It certainly is good to ensure one doesn't regress!

Also, yes, some automation would be great! We're fairly good with our numpydoc format, so some automation should be possible. It may be worth reaching out to the numpy folks who did it.

[nstarman]

Thanks @namurphy! I'm looking forward to seeing how this works in PlasmaPy.
I agree with @mhvk that mypy should be configured on a per-sub package basis. Thankfully this is straightforward to do in mypy's pyproject.toml. The thing Im unsure if it's possible is to automate the detection that a file passed mypy and then enforce that it cannot regress. It's easy enough to manually remove a file from an ignore-list when it's passing, but that requires active maintainer input.
If you're interested to start, I'd be happy to type Cosmology and we can test configuration / tools.

[neutrinoceros]

Would it make sense to start by enabling mypy in pre-commit

To quickly reply to this: mypy is limited if ran from pre-commit because each hook is installed in isolation, while mypy needs to be installed alongside the package being analysed (and most important, that package's dependencies). I would recommend that type-checking be instead integrated with tox/CI

[namurphy]

This is a really helpful and enjoyable discussion — thank you all so much!

The thing Im unsure if it's possible is to automate the detection that a file passed mypy and then enforce that it cannot regress.

Yeah, I'm not sure how to do that automagically. A related approach would be to have mypy ignore particular lines using # type: ignore[specific-error-code] in files that are in the process of being type-checked/mypyified. That way, no new type-checking errors would get added. 🤔

If you're interested to start, I'd be happy to type Cosmology and we can test configuration / tools.

Awesome! I'm wondering if it would make sense to start the typing process by going up the dependency tree, i.e. starting with the subpackages & modules that other subpackages & modules depend on.

To quickly reply to this: mypy is limited if ran from pre-commit because each hook is installed in isolation, while mypy needs to be installed alongside the package being analysed (and most important, that package's dependencies). I would recommend that type-checking be instead integrated with tox/CI

Interesting! Another advantage of using tox & GitHub Actions would be that we could add a note at the end that says something like, "For more information about typing and troubleshooting with mypy, see mypy's typing cheatsheet and this page in Astropy's contributor guide" with the appropriate hyperlinks. Having mypy in pre-commit would also add a performance overhead to pre-commit since it needs to be run on the whole project.

It'd probably be helpful to have a section or page in the contributor guide on this too, which would at least point to the appropriate typing resources and then discuss particular Astropy nuances.

[namurphy]

I don't think we should make contributing harder before we have the basics in place, and have documented where to find the existing astropy types, etc.

Agreed! This is especially true since I've been finding the error messages in mypy a bit hard to understand, in particular because a type error in one line may have been caused by an incorrect annotation in a completely different location.

Or might it be possible to do it on a per-file basis? If it passes mypy on main, it should pass on the PR? Or is this what you meant? It certainly is good to ensure one doesn't regress!

Yes, this is essentially what I'm doing in PlasmaPy/PlasmaPy#2424. Instead of ignoring all subpackages, I'm setting it to ignore the individual files that mypy found errors in. We'll likely be going through the files one-by-one to either fix/add annotations or add # type: ignore comments on particular lines.

We could perhaps start by configuring mypy so that it doesn't require type hints to be present, but rather check that type hint annotations are correct if they are present.

[eerovaher]

A good thing to do sooner rather than later would be to address the remaining violations of the Ruff ANN ruleset:

astropy/.ruff.toml

Lines 6 to 11 in 6568d90

	# flake8-annotations (ANN) : static typing
	"ANN001", # Function argument without type annotation
	"ANN003", # `**kwargs` without type annotation
	"ANN201", # Public function without return type annotation
	"ANN202", # Private function without return type annotation
	"ANN401", # Use of `Any` type

Note that we have configured Ruff to ignore the ANN rules in functions without any annotations and that there already is a pull request for ANN201.

[namurphy]

I just submitted a demo PR in #15794 that takes the approach that I described above and am using in PlasmaPy/PlasmaPy#2424. It gets mypy to pass by ignoring errors on a per-file and per-error basis. It's by no means the only approach, and since there are tradeoffs, it's definitely worth considering other approaches (like in #12971). In any case, it would be a starting point at least!

I also created an issue about #15170 about writing an APE about typing, but I'd suggest we continue this conversation here for the time being.

Thank you again everyone!

[tactipus]

I'm not going to read all this, but I'm going to assume you're gonna need some hands to actually type all these types out. I volunteer.

[namurphy]

I'm in the process of using autotyping to automatically add type hint annotations in PlasmaPy/PlasmaPy#2437 and PlasmaPy/PlasmaPy#2439. I recommend it, though I haven't tried other tools to compare it against.

So far, I'm adding return annotations to special methods like __init__ and __str__, and callables that lack a return statement. Some more detailed thoughts:

• The automated changes are pretty much all low-hanging fruit. The --safe options are pretty reliable, but the --aggressive options will require more carefully going through each of the changes.
• The volume of changes is very large, so I suggest subpackage-by-subpackage PRs to make code review more manageable...starting with the --safe options.
• The changes include adding a lot of -> None annotations to tests. I think this is okay since mypy seemingly only does static type checking on callables that have at least one annotation, and this will let mypy verify that tests do not have a return statement.
• The docs for mypy say that __init__ methods don't need a return annotation so long as one of the parameters to __init__ has an annotation. However, having -> None return annotations ensures mypy will check those methods rather than assuming they're untyped.
• I needed to regenerate the per-file ignores in mypy.ini like in Add mypy configuration file that ignores existing errors on a per-file and per-error basis #15794. Interestingly, this uncovered some new mypy errors in functions that weren't being type checked before because they previously had no annotations.
• I'm wondering if autofixes from the ANN rule set in ruff could be used for this instead.

[neutrinoceros]

• The changes include adding a lot of -> None annotations to tests. I think this is okay since mypy seemingly only does static type checking on callables that have at least one annotation, and this will let mypy verify that tests do not have a return statement.
• The docs for mypy say that init methods don't need a return annotation so long as one of the parameters to init has an annotation. However, having -> None return annotations ensures mypy will check those methods rather than assuming they're untyped.
• I needed to regenerate the per-file ignores in mypy.ini like in Add mypy configuration file that ignores existing errors on a per-file and per-error basis #15794. Interestingly, this uncovered some new mypy errors in functions that weren't being type checked before because they previously had no annotations.

Just wanted to chime in on this and point out that this behaviour can be configured: https://mypy.readthedocs.io/en/stable/config_file.html#confval-check_untyped_defs
So if the goal is to make sure test and __init__ functions are checked, but we don't want to bother actually typing them (I don't even know how a pytest fixture should be typed actually), this can be much more easily done by just setting check_untyped_defs = true, and help reduce the size of diffs ?

[namurphy]

We recently added mypy to PlasmaPy's CI 🎉, and I've been finding it to be pretty good overall. The main pain points that I've found are (1) it's a bit annoying to configure, and (2) the error messages are sometimes a bit hard to understand. For example, sometimes the error message will point to a line, while the problem was actually in an earlier line.

A takeaway lesson from my experience with PlasmaPy is that it is worthwhile to investigate alternatives to mypy. For example, pyright is an open source static type checker developed by Microsoft that is designed for high performance and to work with large projects. Most of the informal discussions I've seen on the World Wide Web tend to favor pyright over mypy, including because of better error messages.

I also found an article on static type checkers that also covers pytype and pyre.

Edited to add: I just tried out pyright and share my initial impressions in PlasmaPy/PlasmaPy#2451. For the moment I'm inclined to stick with mypy, but the next time I go through a file to add type hint annotations in PlasmaPy, I'm going to try using both mypy and pyright so that I can better compare them.

[taldcroft]

Just to add my opinion at this point after a lot of good discussion on implementing static type checking. The original issue description did not touch on adding type checking to CI and here I think we need to be cautious.

I'm a huge fan of type annotation as documentation for the developer. Basically you and your IDE can look at code and quickly see the intended types. This helps the developer and makes it easier to make quality code contributions.

I am worried that enforcing static type checking, even in a limited way, will make it substantially harder for developers to contribute code.

This is based on my experience when trying to enable pyright "basic" checking in VS code (not even strict) on some small pieces of code that have basic type annotations (at the level which provide useful documentation). Basically what happens is that pyright shows a lot of warnings which are not related to code correctness. Many of them come from upstream untyped packages, which is basically a show-stopper because that means my own code always has a bunch of spurious warnings and those hide real warnings that I should be noticing.

Beyond that, trying to fix them all ends up taking a lot of time, and sometimes they should not even be fixed. Python is still fundamentally a dynamic language after all.

A simple example is a function that requires a str argument and the first thing it does is validate that argument and raise an exception otherwise. Then I use this function in a try/except block with an argument that is str | Path and expect that for a Path it raises an exception. The type checker complains about this. Maybe there is a way around this but I fear this is just the tip of an iceberg of barriers to development.

[eerovaher]

What @taldcroft describes seems to be very similar to what I referred to as tier 1 in an earlier comment of mine. One of the reasons implementing that level of typing should be uncontroversial is that it does not require us to settle on using any particular type checker.

I will also point out that it is not at all obvious that a type checker that might be the best for an untyped code base, like astropy is right now, would also be the best for an almost entirely typed code base that I hope astropy will be. We might want to apply some type checker in a CI job eventually, but it is far too early to discuss which one it should be.

Right now the best thing to do would be to simply start adding type annotations without worrying too much about configuring any type checkers for a CI job. If developers find some tool or another to be useful then those tools can be used without having to run them in CI.

PS: @taldcroft, do you know about type guards?

[taldcroft]

@eerovaher - about type guards, I had seen them previously but have not used them. I feel like this is a case where typing starts to drive code changes that may be undesirable. For instance replacing a very-fast inline statement like if not isinstance(arg, str): with a dedicated (trivial) type guard function and a much slower function call to a type guard function. This feels counter to the foundation of typing that it does not impact runtime behavior of code.

In my particular case I don't think a type guard would even help to remove that warning. The first function is typed def func1(arg: str) because any other type will raise a Func1Error exception. In addition if arg does not parse in a particular format then Func1Error is also raised. Then it gets called in:

def func2(arg: str | Path) -> SomeType:
    try:
        out = func1(arg)
    except Func1Error:
        out = something_else(arg)

The type checker will always complain that I'm passing the wrong type to func1. It is important that func2 doesn't need to know anything about the func1 requirement for arg, just whether or not it returned a result or failed. Is there a way to write this in a way that works with type checking?

I do appreciate that typing and the sorts of warnings that are revealed tend to drive us to generally better code. The biggest lesson I am learning is being specific about return types.

[eerovaher]

@taldcroft, we are getting somewhat off topic, so I'll be very brief:

from typing import cast
...
def func2(arg: str | Path) -> SomeType:
    try:
        out = func1(cast(str, arg))
    except Func1Error:
        out = something_else(arg)

[namurphy]

Thank you all for continuing this discussion!

I'm a huge fan of type annotation as documentation for the developer. Basically you and your IDE can look at code and quickly see the intended types. This helps the developer and makes it easier to make quality code contributions.

Agreed! The biggest part of my motivation for adding type hint annotations to astropy.units is to improve the developer experience for downstream packages like PlasmaPy.

I am worried that enforcing static type checking, even in a limited way, will make it substantially harder for developers to contribute code.

This is the biggest worry I have about enabling mypy in CI for PlasmaPy, and I've been working on some mitigation strategies (like discussing type hint annotations in our contributor guide, and mentioning type hints in the comment that gets posted to every PR). I'll be happy to share which strategies end up working best for us. Given the size of the Astropy community, it's probably worth writing an APE about the process of adding type hint annotations.

To come at this from a different angle, a friend pointed out that adding type hint annotations could potentially be a great first issue for new contributors, in particular if we can point to a good discussion of the process of adding type hint annotations.

...pyright shows a lot of warnings which are not related to code correctness. Many of them come from upstream untyped packages...

The way I handled this situation for PlasmaPy was to configure mypy.ini to not report errors when particular upstream packages lacking type hints. I'd be happy to do this for Astropy too.

[taldcroft]

Thanks @eerovaher and @namurphy - I'm learning a lot and this seems productive! The experience from plasmapy is quite valuable.

About a document for adding type hint annotations, I might suggest a wiki page rather than APE. APE's are quite difficult to update, whereas I think a process document / tutorial on add type annotations should be a living document and get frequent updates as we gain experience and as typing continues to evolve. The example from @eerovaher would be a perfect thing to quickly add to a typing hints guide without much friction.

[namurphy]

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!

[namurphy]

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.

[nstarman]

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

[taldcroft]

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.

Footnotes

1. 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. ↩

[taldcroft]

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.

[eerovaher]

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.

[namurphy]

@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.

[tactipus]

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.

[nstarman]

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

[tactipus]

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.

[mhvk]

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"/.

[namurphy]

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.

[namurphy]

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.

[mhvk]

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:

1. 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.
2. 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!

[mhvk]

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.