Skip to content

FIX: Allow eyetrack channels to be used with plot_compare_evoked#12190

Merged
larsoner merged 7 commits intomne-tools:mainfrom
scott-huberty:eyetrack_compare_evokeds
Nov 25, 2023
Merged

FIX: Allow eyetrack channels to be used with plot_compare_evoked#12190
larsoner merged 7 commits intomne-tools:mainfrom
scott-huberty:eyetrack_compare_evokeds

Conversation

@scott-huberty
Copy link
Copy Markdown
Contributor

@scott-huberty scott-huberty commented Nov 9, 2023 •
edited
Loading

Per this discussion post:

https://mne.discourse.group/t/mne-viz-plot-compare-evokeds-for-eyetracking/7816

It seems that eyetrack channels are not an accepted channel type for plot_compare_evokeds. Off the top of my head I can't think of a good reason for why they shouldn't be accepted, so I've gone ahead and implemented a quick fix. I wouldn't mind a moment to really think about this or discuss it with others, but I'm defending my PhD tomorrow so this thinking won't happen for me until next week 😃

A MWE, which is just a proof of concept (in this example the evokeds aren't informative at all). (requires mne 1.6):

import mne
fpath = mne.datasets.eyelink.data_path() /  "freeviewing"
fname = fpath / "sub-01_task-freeview_eyetrack.asc"
raw = mne.io.read_raw_eyelink(fname)

mne.preprocessing.eyetracking.interpolate_blinks(raw, interpolate_gaze=True)
raw.annotations.rename({"dvns": "natural", "dvss": "scrambled"})  # more intuitive
event_ids = {"natural": 1, "scrambled": 2}
events, event_dict = mne.events_from_annotations(raw, event_id=event_ids)

epochs = mne.Epochs(raw, events =events, event_id=event_dict, tmin=0, tmax=20, baseline=None)

evokeds = dict()

evokeds["natural"] = epochs["natural"].average()
evokeds["scrambled"] = epochs["scrambled"].average()

mne.viz.plot_compare_evokeds(evokeds)

Figure_1

@scott-huberty scott-huberty marked this pull request as draft November 9, 2023 15:15
@mscheltienne
Copy link
Copy Markdown
Member

mscheltienne commented Nov 9, 2023

The real question to me is: should eyegaze and pupil channels be considered as data channels?

I'm defending my PhD tomorrow so this thinking won't happen for me until next week

No hurry, and best wishes!

@scott-huberty scott-huberty changed the title FIX: Allow eyetrack channels to be used this plot_compare_evoked FIX: Allow eyetrack channels to be used with plot_compare_evoked Nov 9, 2023
@mscheltienne
Copy link
Copy Markdown
Member

mscheltienne commented Nov 9, 2023

For the pupil size, that opens another question when comparing evoked/signals from different subjects/recordings. As it's in AU, and the camera distance/illumination/lens can differ + the subject eye size/shape differ, is it actually comparable?

@drammock
Copy link
Copy Markdown
Member

drammock commented Nov 9, 2023

For the pupil size, that opens another question when comparing evoked/signals from different subjects/recordings. As it's in AU, and the camera distance/illumination/lens can differ + the subject eye size/shape differ, is it actually comparable?

zscoring within subject is a typical answer to this worry. Though researchers also do try to control distance to camera/screen (e.g. with a chinrest and forehead bar)

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Nov 9, 2023

The real question to me is: should eyegaze and pupil channels be considered as data channels?

I would say no, they are not "ephys data from the brain" which is what we mean by "data"

https://mne.tools/dev/documentation/glossary.html#term-data-channels

@scott-huberty scott-huberty marked this pull request as ready for review November 10, 2023 15:43
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Nov 10, 2023

Can you ad a tiny test that it plots? So add to mne/viz/tests/test_evoked.py that creates an Evoked from eyelink data and calls plot_compare_evokeds. Doesn't matter if the comparison is meaningful.

Also on main did you get a reasonable error /ike that no acceptable channels types were found, or was it cryptic? If it was cryptic we should also improve it to say "no plottable channel types found, plot_compare_evokeds doesn't support plotting channel types found in data: {inst.get_channel_types(unique=True)} or similar.

@scott-huberty
Copy link
Copy Markdown
Contributor Author

scott-huberty commented Nov 10, 2023

Can you ad a tiny test that it plots? So add to mne/viz/tests/test_evoked.py that creates an Evoked from eyelink data and calls plot_compare_evokeds.

Sure can do.

Also on main did you get a reasonable error /ike that no acceptable channels types were found, or was it cryptic?

Actually it was an assert error.. I can amend it use an if not clause and raise a more informative error

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Nov 10, 2023

Actually it was an assert error.. I can amend it use an if not clause and raise a more informative error

That would be great. I would use inst.set_channel_types(...) to change the channel types to something that can't be plotted. From the looks of it EOG might work for now. Or if you want to pick something more esoteric that we probably never care to plot with plot_compare_evokeds you could go with ias which is internal active shielding information for Neuromag/MEGIN systems.

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Nov 10, 2023

... and actually you even could use this set_channel_types approach to pretend the EEG or MEG sensor data are eyetracking pupi/gaze data for the purposes of the test that should now pass that currently fails on main. Reading actual data would be more lines of code but also more relevant. Up to you :)

@scott-huberty
Copy link
Copy Markdown
Contributor Author

scott-huberty commented Nov 21, 2023

Update. I added a more informative error and a few more lines to test_plot_compare_evokeds, to test that plotting eye-tracking data works, and that an informative error is thrown if the evoked instance only contains channel types that are not accepted by plot_compare_evokeds (i.e. an evoked instance with only EOG channels`).

larsoner
larsoner reviewed Nov 21, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Can you add a line to doc/changes/devel.rst and merge in latest main (or rebase) and then we can merge? Thanks @scott-huberty !

@scott-huberty
Copy link
Copy Markdown
Contributor Author

scott-huberty commented Nov 22, 2023

Oh yes thanks for reminding me!

@larsoner larsoner merged commit 0a9c61b into mne-tools:main Nov 25, 2023
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Nov 25, 2023

Thanks @scott-huberty !

ctrltz pushed a commit to ctrltz/mne-python that referenced this pull request Nov 25, 2023
@scott-huberty @ctrltz
FIX: Allow eyetrack channels to be used with plot_compare_evoked (mne…
0b10a0b 
…-tools#12190)
larsoner added a commit to hoechenberger/mne-python that referenced this pull request Nov 27, 2023
@larsoner
Merge remote-tracking branch 'upstream/main' into neuralynx
13f051e 
* upstream/main:
  Add some omitted dependencies to "full" and "doc" variants (mne-tools#12235)
  Fix pyproject.toml setuptools configuration (mne-tools#12240)
  FIX: Allow eyetrack channels to be used with plot_compare_evoked (mne-tools#12190)
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
@scott-huberty @snwnde
FIX: Allow eyetrack channels to be used with plot_compare_evoked (mne…
76b37e4 
…-tools#12190)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@larsoner larsoner larsoner left review comments

@mscheltienne mscheltienne mscheltienne approved these changes

@drammock drammock Awaiting requested review from drammock drammock is a code owner

@dengemann dengemann Awaiting requested review from dengemann

@agramfort agramfort Awaiting requested review from agramfort agramfort is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@scott-huberty @mscheltienne @drammock @larsoner