Type public Epochs, Evoked, and io members fully by larsoner · Pull Request #14056 · mne-tools/mne-python

larsoner · 2026-07-14T17:49:44Z

Okay this PR diff is big, but it's simple enough at its core:

Add a test that checks that our NumpyDoc-documented docstrings match the types we define in our code. This turned up a few minor issues (fixed).
Fill in type annotations for public interfaces in mne/io, mne/evoked.py, and mne/epochs.py, ensuring the test passes

I had Claude Opus 4.8 work on the test, but I simplified it and checked it. I also had Claude do the drudge-work of putting all those type annotations inline. The test ensures they're correct, though, so this should make review a bit easier (hopefully)!

This adds a little bit of work to future contributions having to type-define two places (docstring + code), but I don't think that's too onerous. Someday if numpy/numpydoc#196 (or numpy/numpydoc#601) lands we could maybe DRY things again.

I think with this scaffolding in place, we're safe to move to typing other modules in a similar fashion (or by using typestubs as a first pass, which @drammock is going to look into IIRC). In the meantime, I suggest we add this since it should make things slightly nicer for our end-users.

I considered pydoclint as a checker, but it is a static tool, so we need to get rid of %/docdict before we could pursue that.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

wmvanvliet · 2026-07-20T16:20:21Z

Wait, MNE-Python is going typed now?

larsoner · 2026-07-20T16:21:53Z

We've discussed it and proposed it in grants but never gotten it funded IIRC

drammock

thanks for tackling this @larsoner. I looked pretty closely at all the readers in io/, the tests, and the utils. io/base.py I skimmed, and epochs.py and evoked.py I skipped.

There are merge conflicts now but once those are gone and you've rebutted/addressed all my comments, +1 for merge.

drammock · 2026-07-23T19:21:08Z

+    input_fname: Path | str | FileLike,
+    eog: list | tuple | None = None,
+    misc: list | tuple | None = None,
+    stim_channel: Literal["auto"] | str | list | int = "auto",


is there a point to saying Literal["auto"] | str instead of just str here (and elsewhere in the diff)?

I actually think it's a tiny bit clearer -- "auto" is a special thing, whereas any other string is a channel name and behaves differently. This is consistent with how our docstring documents it as well

stim_channel : ``'auto'`` | str | list of str | int | list of int

... which is enforced to be consistent the checker!

drammock · 2026-07-23T21:31:25Z

+    s = re.sub(r"\s*\(\s*default[^)]*\)", "", s, flags=re.I)  # (default X)
+    s = re.sub(r"\s*,\s*default[:=]?\s*[^,|]+", "", s, flags=re.I)  # , default X


these should be removed from the docstring sources, IMO. AFAIR our convention is to state the default in the param description prose, not in the type description. Can be a TODO for a follow-up.

Yeah that's a good follow-up, and removing this would be a way to find the violators.

drammock · 2026-07-23T21:33:03Z

+    s = s.replace("~", "")  # drop the sphinx "abbreviate" marker
+    s = re.sub(r"\s*\(\s*default[^)]*\)", "", s, flags=re.I)  # (default X)
+    s = re.sub(r"\s*,\s*default[:=]?\s*[^,|]+", "", s, flags=re.I)  # , default X
+    s = re.sub(r"\s*,?\s*optional\b", "", s, flags=re.I)  # , optional


"optional" is also not something we're supposed to do according to the contrib guide (item 2)

drammock · 2026-07-23T21:35:36Z

+    s = re.sub(
+        r"\b(list|tuple|dict|set) of [\w.]+", r"\1", s, flags=re.I
+    )  # list of X -> list


the . in [\w.]+ should be escaped? Assume it's meant to capture the dot in e.g. "list of numpy.ndarray"

I always have to look this stuff up ! Looks like it should be okay

In a character class (square brackets) any character except ^, -, ] or \ is a literal.

https://stackoverflow.com/questions/19976018/does-a-dot-have-to-be-escaped-in-a-character-class-square-brackets-of-a-regula

drammock · 2026-07-23T21:43:58Z

+    s = re.sub(  # textual Literal["a", "b"] (from string annotations) -> a | b
+        r"\bLiteral\[([^\]]*)\]", lambda m: m.group(1).replace(",", "|"), s
+    )
+    s = re.sub(
+        r"\{([^}]*)\}", lambda m: m.group(1).replace(",", "|"), s
+    )  # {a, b} -> a|b


IIUC, these two will map Literal["a", "b"] or {"a", "b"} to "a"| "b" (note the asymmetric space). Does that matter?

drammock · 2026-07-23T21:45:50Z

+        elif "array" in low or low == "ndarray":
+            atoms.add("array")


this could miss marray if we happen to take in or return masked arrays somewhere

Yeah I expect we might have to adjust this if we add more types

drammock · 2026-07-23T21:50:16Z

+        # free-form docstring types (e.g. "matplotlib colormap | (colormap, bool)")
+        # cannot be matched mechanically, so only validate comparable ones
+        if not doc_atoms or not all(_is_type_like(d) for d in doc_atoms):
+            continue


this feels like a case of "we shouldn't be doing that in the first place". Are there other cases like this besides the interactive colormap one? would it make sense to have an explicit allowlist (so we have something we can work on whittling down)?

drammock · 2026-07-23T21:54:55Z

-            if not fname.startswith("_") and _defined_under(func, name):
-                _check_type_hints(func, cls=None, where=name, incorrect=incorrect)
+    for func, cls in _documented_callables():
+        _check_type_hints(func, cls=cls, incorrect=incorrect)


seems weird that the helper mutates the input incorrect list, and doesn't return anything. Unexpected (but I guess ok as it's not user-facing and only used here?)

drammock · 2026-07-23T21:56:54Z

                                )
            super().generic_visit(node)

+        def visit_If(self, node):


maybe worth a comment that the capital I is intentional / the func name is dictated by the way ast.NodeVisitor works

drammock · 2026-07-23T22:08:56Z


-def read_raw_fieldtrip(fname, info, data_name="data") -> RawArray:
+def read_raw_fieldtrip(
+    fname: Path | str, info: dict | None, data_name: str = "data"


why info: dict | None here instead of info: Info | None?

Preexisting cruft with the docstring ... I'll fix it here though!

larsoner · 2026-07-23T23:33:49Z

Pushed a commit with the allowlist approach (most look pretty easily fixable, but probably better reviewed separately) plus regex and minor comment cleanups/adds. Marking for squash+merge-when-green 🤞

wmvanvliet · 2026-07-24T07:47:28Z

Any recommendations on how to proceed with adding type annotations to other parts of the codebase? When we write new stuff and re-write existing stuff, should we type annotate it?

drammock · 2026-07-24T11:19:34Z

Any recommendations on how to proceed with adding type annotations to other parts of the codebase? When we write new stuff and re-write existing stuff, should we type annotate it?

@larsoner and I are planning to crank through the rest of the codebase now-ish, one submodule at a time, with the help of the docstub package and an LLM. LMK if you want to assist, so we don't duplicate effort

Type public Epochs, Evoked, and io members fully

ba2688c

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

larsoner requested review from agramfort, drammock, mscheltienne and sappelhoff as code owners July 14, 2026 17:49

larsoner added 2 commits July 14, 2026 13:54

FIX: Log

477c069

FIX: autodoc typehints are a pain

f9189a8

drammock reviewed Jul 23, 2026

View reviewed changes

larsoner mentioned this pull request Jul 23, 2026

Remove ", default X" from docstrings #14095

Open

larsoner added 2 commits July 23, 2026 19:06

FIX: Comments

4388332

Merge upstream/main into typed-fully

2322652

larsoner enabled auto-merge (squash) July 23, 2026 23:23

larsoner merged commit 41ae901 into mne-tools:main Jul 23, 2026
29 checks passed

larsoner deleted the typed-fully branch July 23, 2026 23:48

		s = re.sub(r"\s\(\sdefault[^)]*\)", "", s, flags=re.I) # (default X)
		s = re.sub(r"\s,\sdefault[:=]?\s*[^,\|]+", "", s, flags=re.I) # , default X

Uh oh!

Uh oh!

Conversation

larsoner commented Jul 14, 2026

Uh oh!

wmvanvliet commented Jul 20, 2026

Uh oh!

larsoner commented Jul 20, 2026

Uh oh!

drammock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larsoner commented Jul 23, 2026

Uh oh!

Uh oh!

wmvanvliet commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drammock commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wmvanvliet commented Jul 24, 2026 •

edited

Loading