ENH, FIX: Make eyelink occular annotations "channel aware", and call blink annotations "BAD"

[scott-huberty]

Reference issue

This PR is in reference to the discussion in PR #11740 , see this comment

• Note to self: Need to add this PR to the change log

What does this implement/fix?

1. occular annotations ('fixations', 'saccades', 'blinks') are now channel aware.
2. blink annotation descriptions are now prepended with BAD_
3. Tests: I added tests to account for point 1 and 2 above.
4. Clean up: There were a lot of helper functions sitting in mne.io.eyelink, I moved them to the _utils.py file that sits in the same directory.

Example:

import mne
fname = mne.datasets.testing.data_path() / "eyetrack" / "test_eyelink.asc"
raw = mne.io.read_raw_eyelink(fname,
                              create_annotations=["blinks"],
                              find_overlaps=True)
raw.plot(scalings='auto', start=20)
print(raw.annotations[5])

OrderedDict([('onset', 28.551),
             ('duration', 0.08),
             ('description', 'BAD_blink_both'),
             ('orig_time',
              datetime.datetime(2022, 3, 10, 11, 38, 16, tzinfo=datetime.timezone.utc)),
             ('ch_names',
              ('xpos_left',
               'ypos_left',
               'pupil_left',
               'xpos_right',
               'ypos_right',
               'pupil_right'))])

[scott-huberty]

Note: Updated change log and updated the eye-tracking tutorials to mention/display the ch_names annotation key.

EDIT: here: https://output.circle-artifacts.com/output/job/e3c7ced6-00ef-49d6-9acd-bd95a5073da9/artifacts/0/html/auto_tutorials/preprocessing/90_eyetracking_data.html

[larsoner]

Instead let's use convenience functions as much as possible, like

_validate_type(token, (str, 'numeric'), 'token')

Then you can just do your try/except without any type business

[scott-huberty]

sure, to clarify, that would be:

try:
      _validate_type(token, (str, 'numeric'), 'token')
       return True
except ValueError:
       return False

?

[larsoner]

Currently you raise an error for anything other than str, then try to convert to float and return True/False based on floatiness. So I guess it should be:

_validate_type(token, str, 'token')
try:
    float(token)
except ValueError:
    return False
else:
    return True

[larsoner]

... and as a general follow-up comment, please do look at the coverage after your next commit and take a look at the missed (red) lines and see if there is some way to get them covered.

[scott-huberty]

... and as a general follow-up comment, please do look at the coverage after your next commit and take a look at the missed (red) lines and see if there is some way to get them covered.

Will do. I may need to add one or 2 more 1-second test files to mne-testing-data.

Also see my other comment about whether it would be worth taking another pass at this function (to see where things can be tightened up), as part of my GSoC.

[larsoner]

I may need to add one or 2 more 1-second test files to mne-testing-data.

Another option if it's feasible -- since these are ASCII files right? -- is to just use tmp_path and read a test file line-by-line and write it to tmp_path line-by-line modifying (or adding or deleting) a few along the way to make files that would hit any problematic conditions. This avoids the need to add more test files to mne-testing-data and is usually plenty explicit. See for example here where we do it with a binary file (which is harder than ASCII I think):

mne-python/mne/io/edf/tests/test_edf.py

Lines 212 to 225 in 1d776cb

	# Test with number of records not in header (-1).
	broken_fname = tmp_path / "broken.edf"
	with open(edf_path, "rb") as fid_in:
	fid_in.seek(0, 2)
	n_bytes = fid_in.tell()
	fid_in.seek(0, 0)
	rbytes = fid_in.read()
	with open(broken_fname, "wb") as fid_out:
	fid_out.write(rbytes[:236])
	fid_out.write(b"-1 ")
	fid_out.write(rbytes[244 : 244 + int(n_bytes * 0.4)])
	with pytest.warns(RuntimeWarning, match="records .* not match the file size"):
	raw = read_raw_edf(broken_fname, preload=True)
	read_raw_edf(broken_fname, exclude=raw.ch_names[:132], preload=True)

about whether it would be worth taking another pass at this function (to see where things can be tightened up), as part of my GSoC.

Yes code coverage and using better / more pythonic / more MNE-Pythonic coding practices would definitely fit! Maybe a good thing to do when you're waiting on any slow-to-respond devs to get around to reviewing your code :)

[larsoner]

... so in other words you don't have to spend hours trying to improve coverage here. We can leave it as a longer-term goal on your TODO list somewhere

[scott-huberty]

I may need to add one or 2 more 1-second test files to mne-testing-data.

Another option if it's feasible -- since these are ASCII files right? -- is to just use tmp_path and read a test file line-by-line and write it to tmp_path line-by-line modifying (or adding or deleting) a few along the way to make files that would hit any problematic conditions.

Sounds good I can give that a shot!

Yes code coverage and using better / more pythonic / more MNE-Pythonic coding practices would definitely fit! Maybe a good thing to do when you're waiting on any slow-to-respond devs to get around to reviewing your code :) ... We can leave it as a longer-term goal on your TODO list somewhere

Ha free time is always short but I I can start a separate branch and pick at it over the next few weeks. For now I'll take a pass and look for some low hanging fruit to increase the coverage. I can open a ticket regarding general improvements for read_raw_eyelink, with a goal to finish it at some point this summer!

[scott-huberty]

@larsoner just a heads up, since all ocular annotations are channel aware now, I will remove the "eye" name from their description (i.e. all blink annotations will be BAD_blink now, instead of BAD_blink_L etc.).

[larsoner]

M1 failure is unrelated, thanks @scott-huberty !