Fix interpolate_blinks ignoring non-BAD_blink match descriptions (#13880)#13940
Open
gaoflow wants to merge 2 commits into
Open
Fix interpolate_blinks ignoring non-BAD_blink match descriptions (#13880)#13940gaoflow wants to merge 2 commits into
gaoflow wants to merge 2 commits into
Conversation
…-tools#13880) `interpolate_blinks` accepts a `match` parameter (str or list of str) and correctly filters `blink_annots` by it, but `_interpolate_blinks` derived the segment start/stop times from a hardcoded `_annotations_starts_stops(raw, "BAD_blink")` query. As a result, annotations whose description was in `match` but did not start with `BAD_blink` (e.g. `BAD_NAN` from `annotate_nan`) were silently dropped, and the `zip(blink_annots, starts, ends)` could misalign. Compute the start/stop times directly from the `blink_annots` that were passed in, using the same onset-sync and sample rounding as before, so the default `BAD_blink` behaviour is unchanged while every matched annotation is now interpolated over.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference issue
Fixes #13880.
What does this implement/fix?
mne.preprocessing.eyetracking.interpolate_blinksdocuments amatchparameter (str | list of str) selecting which annotation descriptions to interpolate over. The public function correctly filters the annotations bymatch:but the private
_interpolate_blinksthen derived the segment start/stop times from a hardcoded query:So any matched annotation whose description does not start with
BAD_blink— e.g.BAD_NANcreated bymne.preprocessing.annotate_nanand passed viamatch=["BAD_blink", "BAD_NAN"]— was silently ignored. Worse, becauseblink_annots(filtered bymatch) andstarts/ends(from the hardcoded"BAD_blink") could have different lengths/orderings, thezipcould truncate and pair an annotation with the wrong start/stop time.Fix
Compute
starts/endsdirectly from theblink_annotsthat are passed into_interpolate_blinks, using the same onset-sync (_sync_onset) and sample rounding (time_as_index(..., use_rounding=True)) as before. This keeps the defaultmatch="BAD_blink"behaviour bit-for-bit identical (same annotations, same rounding) while ensuring every annotation inmatchis interpolated and that start/stop times stay aligned withblink_annots.Verification
Synthetic pupil
Rawwith aBAD_blinkgap and a separateBAD_NANgap:match=["BAD_blink", "BAD_NAN"], only theBAD_blinkgap was filled; theBAD_NANgap stayed at 0.match="BAD_blink", only the blink gap is filled and the other is left untouched (parameter is now respected both ways).Added a non-data-gated regression test
test_interpolate_blinks_respects_matchcovering both directions (passes with the fix, fails without it).