Conversation
SayanDey322
requested review from
adam2392,
agramfort,
dengemann and
mscheltienne
as code owners
| fit_params: dict | None = None, | ||
| max_iter: int | Literal["auto"] = "auto", | ||
| allow_ref_meg: bool = False, | ||
| verbose: Any = None, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@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).
There was a problem hiding this comment.
I think mne.typing would make sense
3 participants
Reference issue (if any)
What does this implement/fix?
Additional information