Add return type hints to read_raw_*(), read_epochs(), read_annotations()
#12296
Conversation
| Returns | ||
| ------- | ||
| annot : instance of Annotations | None | ||
| annot : instance of Annotations |
There was a problem hiding this comment.
we actually raise if annot is None
hoechenberger
requested review from
agramfort,
dengemann,
drammock,
larsoner,
mscheltienne and
rob-luke
as code owners
|
Thanks for the fixes, Eric |
|
Is it really necessary have separate classes for each file format? |
|
Yes based on how EDIT: subclassing of BaseRaw works |
|
OK. But really we could subclass |
Every reader does currently use the same subclass, |
|
No, they use |
Sorry what I mean is that those are all subclasses of BaseRaw, and that they all have the same super class |
|
Local doc build is green and looks good, marking for merge when green, thanks @hoechenberger ! |
Yes, but what I meant was, why can't they all just be |
|
|
||
| @verbose | ||
| def read_raw_curry(fname, preload=False, verbose=None): | ||
| def read_raw_curry(fname, preload=False, verbose=None) -> "RawCurry": |
There was a problem hiding this comment.
question from a type-hints n00b: why is "RawCurry" a string here? Some of the other type hints like RawEDF are not strings, but references to the Class object.
There was a problem hiding this comment.
See my top comment:
"I had to annotate via strings (names of the classes) in many places because the reader functions appeared before the actual class definition, i.e. I had to make a forward reference."
It's ugly, but I wanted to avoid having to re-order the code.
Thoughts on this?
There was a problem hiding this comment.
oops, sorry! I did read the PR description but somehow my eyes skipped right over that sentence. Let's leave it as-is for now, someone can do the copy-paste reordering later in a commit that we can add to .git-blame-ignore-revs
|
Disabling auto merge while waiting for feedback from @drammock |
Also clean up a little after mne-tools#12296, where I accidentally put a few return types in quoation marks that were, in fact, no forward references. Also updated the changelog.
…ations()` (mne-tools#12296) Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
4 participants
This is a follow-up to #12250
There, I suggested we don't need to add return type hints to
read_raw_*(), because Pylance infers the return type for those functions. Now, working on type stubs, I realized that as soon as stubs for a function are present, Pylance won't infer the return type from the implementation anymore. Hence, this forces us to add return types, or else the UX will degrade in the presence of stubs.I therefore added return type hints for
read_raw_*()read_epochs()read_annotations()(for which I also corrected the docstring, which stated an incorrect return type)I had to annotate via strings (names of the classes) in many places because the reader functions appeared before the actual class definition, i.e. I had to make a forward reference.
@larsoner I had to disable MyPy's
stricttesting, because withstrictit reports errors in functions and classes that are not even annotated yet. I'd be +1 for keepingstrict=false, but feel free to take a look yourself if you have time, thanks!