Skip to content

Type Hints Issue fixed #13603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SayanDey322 wants to merge 1 commit into
base: main
Choose a base branch
from
+27 −27
Open

Type Hints Issue fixed #13603

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions mne/preprocessing/ica.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from inspect import Parameter, isfunction, signature
from numbers import Integral
from time import time
from typing import Literal
from typing import Any, Literal

import numpy as np
from scipy import stats
Expand Down Expand Up @@ -437,15 +437,15 @@ class ICA(ContainsMixin):
@verbose
def __init__(
self,
n_components=None,
n_components: int | float | None = None,
*,
noise_cov=None,
random_state=None,
method="fastica",
fit_params=None,
max_iter="auto",
allow_ref_meg=False,
verbose=None,
noise_cov: Covariance | None = None,
random_state: Any = None,
method: Literal["fastica", "infomax", "picard"] = "fastica",
fit_params: dict | None = None,
max_iter: int | Literal["auto"] = "auto",
allow_ref_meg: bool = False,
verbose: Any = None,
Copy link
Contributor

@scott-huberty scott-huberty Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While Any would help this pass a type checker, IMO it is not a very helpful type annotation for end users, and more to the point, I don't think it is correct here? The Valid arguments for the Verbose parameter are actually constrained:

  • None,
  • True, False,
  • "INFO", "WARNING" "ERROR", "CRITICAL"
  • an integer like 0, 1, 2, etc.

Maybe we can use a TypeAlias? But AFAIK, we have not created any type aliases in MNE-Python yet, so I would like other devs to weigh in here first.

Copy link
Member

@larsoner larsoner Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a TypeAlias would be good here and I would simplify to

VerboseT: TypeAlias = None | bool | Literal["debug", "info", "warning", "error"] | int

maybe we can do away with the upper-case variants for now

Copy link
Contributor

@scott-huberty scott-huberty Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larsoner that sounds good to me. Should we create a dedicated typing module ie. mne.utills.typing and define this there? Or is it too soon to create a dedicated module (given that this is the first TypeAlias).

Copy link
Member

@larsoner larsoner Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mne.typing would make sense

):
_validate_type(method, str, "method")
_validate_type(n_components, (float, "int-like", None))
Expand Down Expand Up @@ -594,16 +594,16 @@ def _repr_html_(self):
@verbose
def fit(
self,
inst,
picks=None,
start=None,
stop=None,
decim=None,
reject=None,
flat=None,
tstep=2.0,
reject_by_annotation=True,
verbose=None,
inst: BaseRaw | BaseEpochs,
picks: Any = None,
start: int | None = None,
stop: int | None = None,
decim: int | None = None,
reject: dict | None = None,
flat: dict | None = None,
tstep: float = 2.0,
reject_by_annotation: bool = True,
verbose: Any = None,
):
"""Run the ICA decomposition on raw data.

Expand Down Expand Up @@ -2176,15 +2176,15 @@ def find_bads_eog(
@verbose
def apply(
self,
inst,
include=None,
exclude=None,
n_pca_components=None,
start=None,
stop=None,
inst: BaseRaw | BaseEpochs | Evoked,
include: list | None = None,
exclude: list | None = None,
n_pca_components: int | None = None,
start: int | None = None,
stop: int | None = None,
*,
on_baseline="warn",
verbose=None,
on_baseline: str = "warn",
verbose: Any = None,
):
"""Remove selected components from the signal.

Expand Down