feat: add support for multiple recording specs per file to gaze.from_asc()

[saphjra]

Description

Record all the tracked eye sides in the metadata to fix the issue #875

Implemented changes

Insert a description of the changes implemented in the pull request.
I modified the code in parsing.py to record every time it can be matched in the message line of the asci files

• Added Regex for Tracking Eye Information in the asci file
• added a condition to match the pattern multiple times and add it to a list
• created an additional pre_processed_metadata key : tracked_eyes, where the list is stored

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?

I ran it on the to ascii converted ch1hr007.edf of the pilot-hr-1-zh data to verify the output, but no further testing has been done so far

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 (70b435a) to head (0267ad1).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #887   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           74        74           
  Lines         3372      3380    +8     
  Branches       594       595    +1     
=========================================
+ Hits          3372      3380    +8     

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

[dkrako]

Great, thanks a lot for the submission!

I would like to increase the scope of this PR to not only account for changes of tracked eyes but also changes of the overall recording configuration. They are all included in the same line and can be matched with a single regex.

Apart from this, please put tests into https://github.com/aeye-lab/pymovements/blob/feature/metadata/tests/unit/utils/parsing_test.py

You can test it two ways: either create a new test file with changing configs or just specify a string like it is done for ASC_TEXT to create a file on the fly.
(Test include only one recording config line at the moment)

[saphjra]

I implemented your suggestions, However. a new problem arise: What value should be given  _calculate_data_loss(
    blinks=blinks,
    invalid_samples=invalid_samples,
    actual_num_samples=actual_number_of_samples,
    total_rec_duration=total_recording_duration,
    sampling_rate=sampling_rate,
) function as sampling_rate. At the moment the _check_sampling_rate() function returns the sampling_rate of the first matched recording_config, However it should probably check if the sampling rate is consistent, raise a warning if not, and probably either declines to compute the data loss, or than takes an average or something similar.

[dkrako]

I think it's sufficient for now to just check for consistency and raise a warning if it's inconsistent.

We can improve on that in a follow-up. Moreover, the logic for calculating data loss will be moved away from this module into the measure module. This way users will be able to calculate these measures on any GazeDataFrame not just when parsed via from_asc().

[dkrako]

these added date strings aren't really necessary, right?
It really doesn't matter in this case and you don't need to revert them, but usually I would advise to avoid changes to existing test logic, e.g. changing test values.

Your other changes here, like adding documentation or changing test ids, are of course the spirit that we need! 🥇

[saphjra]

Thanks for all your Feedback 😄

The thing is, without this additional line, the metadata will be empty, since the MSG line gets parsed into the recording_config and not metadata anymore. However if metadata is empty/ none the code will raise a warning and all the tests, where I added the Date line, will fail due to that.

line 367
"""
if not metadata:
raise Warning('No metadata found. Please check the file for errors.')
"""
So I figured, I add a line that should be present in any dataset, which gets parsed by the metadata.

Is there a better way, to solve this?

[dkrako]

Great, thanks a lot for your update!

Apart from the comments I left, there is the issue that #884 is now conflicting with your PR. We will probably merge #884 first, as it's close to finished.

We will then need to work out how to integrate this PR here into the new functionality from that PR.

I would probably suggest to do something similar as it's done with the screen distance, which can be statically defined in Experiment, but can also be dynamically defined as a column in GazeDataFrame.

Let's think about how to work this issue out and discuss that next week.
Until then, the work on the other comments should be straight forward.