feat: parse eyelink events

[saeub]

Description

Parse fixation, saccade, and blink events detected by EyeLink into an EventDataFrame and add them to the GazeDataFrame returned by from_asc().

Partial fix for #893, alternative implementation to #942.

Implemented changes

• Parse fixations and saccades
• Add events to EventDataFrame (with names fixation_eyelink etc.)
• Add additional columns at start of event (e.g. trial information)
• Fix event duration mismatch: https://www.sr-research.com/support/thread-9411.html
• Refactor or remove old blink logic (e.g. data loss ratio, blinks in metadata dictionary)
• Make event parsing optional

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change is or requires a documentation update

How Has This Been Tested?

• Add fixations and saccade events to test_parse_eyelink
• Test with real ASC file

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have checked my code and corrected any misspellings

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b893eb7) to head (51bebe3).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #945   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           78        78           
  Lines         3560      3596   +36     
  Branches       629       639   +10     
=========================================
+ Hits          3560      3596   +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dkrako]

Great work so far!

I would like to realize #973 before merging this, so we would avoid the breaking change.

I'm indifferent to removing the blink related things from the metadata dictionary.

I would probably leave it there for backwards compatibility and remove it when removing utils.parsing as planned in #973

[dkrako]

* [ ]  Add shortcut property to filter fixations, saccades, and blinks (as proposed in [Added Parsing Of BlinksFrom ASC To Data Frame #942 (review)](https://github.com/aeye-lab/pymovements/pull/942#pullrequestreview-2610335217))

I created #968 for this, so you can ignore this issue for this PR.

[saeub]

I would probably leave it there for backwards compatibility and remove it when removing utils.parsing as planned in #973

@dkrako Does this mean that we should keep the blink-related metadata only in the dictionary returned by utils.parsing.parse_eyelink(), but remove it from the metadata dictionary returned by gaze._utils.parsing.parse_eyelink()?

[dkrako]

I would probably leave it there for backwards compatibility and remove it when removing utils.parsing as planned in #973

@dkrako Does this mean that we should keep the blink-related metadata only in the dictionary returned by utils.parsing.parse_eyelink(), but remove it from the metadata dictionary returned by gaze._utils.parsing.parse_eyelink()?

You convinced me in the meeting two weeks ago, that this wasn't really documented and so probably nobody has even used this. I think it's ok to treat this metadata dict as a very experimental feature which is expected to break in the future, as long as we provide an improved functionality (just as you did in this PR).

[dkrako]

regarding your comment in our minutes:

the only thing left to decide is how to handle EyeLink's weird way of calculating event durations (https://www.sr-research.com/support/thread-9411-post-36595.html); should we use EyeLink's onset/offset timesteps for the events, or move the onset/offset timestamps to match EyeLink's event durations

I'll just write down some of my thoughts:

Unfortunately we didn't really documented well how we define the offset of an event. We have to improve the documentation there.

Judging from the event detection methods, the offset of an event is defined as the first sample that is not in the event any more. So the offset is exclusive (similar to how python range() works).

If I understand correctly, then EyeLink defines the offset differently: it's the last sample of the event. That's why they increase the duration by one sample-length.

Example event: onset=10, offset=11

in pymovements this would lead to a duration of 1.

In contrast EyeLink assumes the offset to be the last event sample (e.g. inclusive), so the same event would be encoded as onset=10, offset=10
that's why they need to add a complete sample-length to the duration.

This is related to this comment by @behinger

I'm not yet sure if you include the events detected by eyelink somewhere, just started checking out the tool. But the automatically detected event-timestamps are rounded down without any way to recover higher precision. This is really a minor thing, as the temporal accuracy to detect such events will never be <1ms - but maybe something to add to a docstring

In conclusion I would say, that we should add a single sample-length to the offset value to counteract the rounding down.

But I'm not fully sure if you'd also expect that, so I'm very open to different points of view @SiQube @prassepaul

[saeub]

In conclusion I would say, that we should add a single sample-length to the offset value to counteract the rounding down.

I agree, this is probably the cleanest way to resolve this.

But in that case, I would suggest finishing #887 first, since the sample rate might change within the ASC file, and we need the sampling rate to calculate a sample length.