POC: Add type annotations to table.py

[taldcroft]

Description

This is a proof of concept for adding type annotations to astropy modules.

For this PR, supporting static type checking is an explicit non-goal.

Claude prompt

Next project is to add type annotations for the table subpackage. The goal here is to make the code easier to read and edit for humans, for instance providing type hints or docstrings for variables in the code with a hover-over.

The minimal way to include type annotations is looking at docstrings, but they may be incomplete. Importantly, do not try to be rigorously complete since this code base has a long history and some APIs are meant to be flexible by allowing a wide range of inputs.

Do not do anything to test modules. As a pathfinder just start with the table.py module and see how that goes.

This was generated entirely by Claude Opus 5.

• 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]

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.

[Copilot]

Pull request overview

This PR is a proof-of-concept pass adding Python type annotations to astropy.table.table (notably Table and related helpers) to improve readability and editor/tooling support across the table subpackage.

Changes:

• Added from __future__ import annotations and a TYPE_CHECKING typing block with local type aliases intended to describe table/column-like inputs.
• Added extensive type annotations to Table, QTable, and supporting classes/functions (method parameters, returns, and some instance attributes).
• Introduced/updated a few signatures to use Self, Literal, and more specific collection/NumPy typing constructs.

Suppressed comments (2)

astropy/table/table.py:307

• TableColumns.__getitem__ handles 0-d integer np.ndarray indices (see the branch checking isinstance(item, np.ndarray)), but the new type annotation does not include np.ndarray. This makes the annotation inconsistent with actual accepted inputs.

    def __getitem__(
        self, item: str | int | np.integer | tuple[str, ...] | slice
    ) -> ColumnLike | TableColumns:

astropy/table/table.py:2000

• show_in_browser still uses a mutable default for jskwargs ({"use_local_files": False}). This default dict is shared across calls and can be mutated by callees (e.g., write() or JSViewer), leading to cross-call leakage. Prefer jskwargs: Mapping[str, Any] | None = None and set the default dict inside the method body.

        max_lines: int = 5000,
        jsviewer: bool = False,
        browser: str = "default",
        jskwargs: Mapping[str, Any] = {"use_local_files": False},
        tableid: str | None = None,

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[taldcroft]

The comment is out of scope for adding type annotations.

[taldcroft]

I believe _mask needs to be array-valued because Table always has a length.

[neutrinoceros]

Thanks for including the prompt you used. I was going to ask for it as a requirement for a review. I'll try to make a first pass soon. Ideally I should be able to get a sense of wether the approach is sensible in a matter of minutes, but I also expect I'll need to read this very carefully before I can greenlight it, possibly spending more time in review than it would have taken me to do it manually.

[taldcroft]

@neutrinoceros - this PR is explicitly a proof of concept draft and not ready for formal review. If you want to do a quick review of part of this PR to estimate level of effort that could be a useful point for the coordination meeting. We could then scale that up to estimate what it would cost for human review after adding type annotations to all of astropy.

Overall, remember that adding typing is not on the astropy roadmap and no funding has been allocated to support such work. You might want to check with the finance committee to see if spending more than an hour or two on this review is consistent with your current contract statement of work.

[TallJimbo]

Functions that return unions of types (as is very common with __getitem__) almost always need typing.overload to work well, so a type checker can reason about how you get one result type with a slice, but a different one with an int, etc.

In fact, in cases where the person writing the calling code knows which return type they'll be getting back reliably, but there isn't a way to indicate that to a type checker with overload, I think it's better to just use Any as the return type - otherwise everybody who calls that function has to wrap it in typing.cast or do an assert isinstance(...) or something, even if their own typing is totally valid. IMO union return types should really only be used in cases where the caller can't really know what type they'll get back and can be reasonably expected to stick to duck-typing or do isinstance checks on it.

Unfortunately I think this is going to end up limiting some of the utility of static typing in Astropy, especially when Quantity is involved (since a Quantity can be a scalar or an array, and the user typically knows which of those they have, but a type checker can't). But I'd definitely advocate for limited typing that uses Any liberally over union usage that breaks type-checked callers.

[timj]

In some cases for duck-typing you can specify a Protocol instead of an explicit type so that downstream has an idea of the methods that need to be supported to act like the duck.

[neutrinoceros]

assert isinstance(...)

note that assert statements do not narrow types; in fact they are completely ignored by type checkers, because they are not guaranteed to be executed at runtime (python -O...)

In some cases for duck-typing you can specify a Protocol

I suggest we try to avoid the expression "duck typing" entirely in these discussions because it's horrendously overloaded and it's generally hard to tell exactly what a person mean by it. In this instance I believe you are referring to structural stubtyping ?

[taldcroft]

From my perspective (certainly for table, time, io.ascii subpackages), supporting static type checking is an explicit non-goal. I recognize that there are differences of opinion and discussion with real world examples helps guide our decisions.

That said, as this thread immediately shows, as soon as you include eventual static type checking as a goal then things get complicated and progress stalls. I personally have no interest in discussing fine points of typing. My priority is fixing bugs or adding features. My opinion is that supporting static typing would require a level of effort and funding that is inconsistent with available project resources.

IMO, an achievable goal for type annotations is to add them mechanically and accept "good but not perfect". This means annotations that look roughly like our current docstrings and help humans navigate and develop the code.

[taldcroft]

To add perspective, the level of effort for this PR would need to scale up by about a factor of 50 to cover all of astropy.

[taldcroft]

I spent a few minutes getting Claude to explain typing.overload to me and now I agree that refining this particular signature would be of value to humans.

At the same time, this entire process was still somewhat labor intensive and does start opening questions of making the code itself start looking more messy (see below). Not a stopper but worth consideration.

For me this suggests a two-phase strategy:

1. Mechanical addition of type annotation that are consistent with docstrings and no detailed review.
2. Gradual improvements that can be done as needed and based on available resources.

from typing import overload

class MyTable:
    @overload
    def __getitem__(self, key: int) -> Row: ...
    @overload
    def __getitem__(self, key: slice) -> Table: ...
    @overload
    def __getitem__(self, key: str) -> Column: ...

    def __getitem__(self, key):
        # actual implementation handles all cases
        ...

[timj]

I'm obviously biased since I've been using type annotations for years but those annotations you are saying add clutter instead add clarity to me as I'm looking at the code. It's now really obvious to me how the three variants behave.

[taldcroft]

I pushed a commit to add typing.overload in two __getitem__ methods.

[neutrinoceros]

note that assert statements do not narrow types; in fact they are completely ignored by type checkers, because they are not guaranteed to be executed at runtime

They do for static type checkers,

Oh, I missed that. Mypy must have added this while I wasn't looking. Meanwhile I've taken on to never rely on assert for narrowing, so I don't know wether other type checkers support this.

[neutrinoceros]

I'm obviously biased since I've been using type annotations for years but those annotations you are saying add clutter instead add clarity to me as I'm looking at the code. It's now really obvious to me how the three variants behave.

Strong +1 here. It's more about learning how to read them than annotations being intrinsically unreadable.

[taldcroft]

Claude highlighted one caveat here that IMO highlights the difficulty of "precise" type annotations. Here the return type depends not just on the type of the input but the shape:

For the edge case of a 0-d integer np.ndarray (e.g. t[np.array(3)]) actually returns a Row at runtime, but I bucketed all np.ndarray under the Self-returning overload since that's the most common usage (boolean masks, index arrays) and the pre-existing flat annotation didn't disambiguate this case either — so no precision was lost, just not gained for that one edge case.

[neutrinoceros]

np.ndarray shouldn't be used as an annotation by itself (I recall it was actually frowned upon): it's really a generic (as are list and tuple), parametrized by the shape and the contained dtype

[neutrinoceros]

In these cases where you want to reach for list | tuple | ndarray, you probably want numpy.typing.ArrayLike instead, which is both shorter and more accurate

[taldcroft]

In these cases where you want to reach for list | tuple | ndarray, you probably want numpy.typing.ArrayLike instead, which is both shorter and more accurate

ArrayLike includes scalars, which is not what we want here.

[neutrinoceros]

Missed that. I guess that's where negation and intersections would be handy as part of the typing grammar, but we're not there yet.