feat: Store metadata from ASC in experiment metadata

[saeub]

Description

Put the metadata parsed in parse_eyelink() into the Experiment, Screen, and EyeTracker classes, and check for mismatches compared to user-provided metadata. Create additional attributes for metadata that is not exposed yet (e.g., calibrations and validations).

Fixes #868.

Related to #875.

Implemented changes

• Store metadata from ASC file in corresponding attributes of Experiment object
• Raise error if there are conflicts with existing values in Experiment
• Store remaining metadata in private attribute GazeDataFrame._metadata to revert breaking change introduced in feat!: Custom patterns for parsing logged metadata in ASC files #767

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?

Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration

• test_from_asc_fills_in_experiment_metadata
• test_from_asc_detects_mismatches_in_experiment_metadata

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 (3cdd3e8) to head (ba65c5b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #884   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           74        74           
  Lines         3372      3419   +47     
  Branches       594       613   +19     
=========================================
+ Hits          3372      3419   +47     

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

[dkrako]

Great, thank you so much for your work! I have some minor change requests, but apart from these the PR is in very good shape.

I would like to include this PR in our upcoming release. This way we avoid the breaking change in the from_asc() interface.
@SiQube @prassepaul What do you think of further delaying the release until this is merged?

[dkrako]

Oh, my review did not take account of the changes you just pushed an hour ago. I hope this is not too confusing.

[saeub]

@dkrako Thanks! Yes, it seems like we were working in parallel :D Working on the remaining changes now.

Originally, we also wanted to store the calibrations and validations in the GazeDataFrame somehow (which is the main thing still missing if metadata is not returned from from_asc()). Should I create another PR for that, so we can merge this one sooner?

[dkrako]

@dkrako Thanks! Yes, it seems like we were working in parallel :D Working on the remaining changes now.

Originally, we also wanted to store the calibrations and validations in the GazeDataFrame somehow (which is the main thing still missing if metadata is not returned from from_asc()). Should I create another PR for that, so we can merge this one sooner?

Yes let's store the calibrations and validations in a follow-up PR.

As the metadata is not returned anymore, you could add a simple line in the from_asc() method right before you return the gaze_df, where you store the metadata in a undocumented private field, for example like this:

gaze_df._metadata = metadata

This way a (both very informed and impatient) user would still be able to use from_asc() and get the metadata.
It's a bit hacky, but that would only be a temporary solution and which would be improved in the follow up PR.
I would keep the field private and undocumented such that we can silently remove it in the next version.

[SiQube]

only some minor comments and questions

[SiQube]

[saeub]

@SiQube I don't understand how this could happen, but it seems like merging #897 broke the test coverage here? Any ideas?

[SiQube]

no its a stupid bug the 0.06 come from the <py3.12 >=py3.12 ...

[saeub]

@SiQube Found this error in the logs of the Codecov action:

error - 2024-11-14 15:48:42,696 -- Report creating failed: [{"input":{"branch":"saeub:metadata-parsing","commit_id":"02b51a67965213162017889e3bbc054b4cc1fafe","repo_id":16278865,"repo_name":"pymovements"},"loc":["id"],"msg":"Field required","type":"missing","url":"https://errors.pydantic.dev/2.4/v/missing"}]

The upload only succeeded for py311-macos, which is why the >=3.12 part isn't covered.

EDIT: I just noticed that a new version of the codecov upload action was released as our builds started failing. I suspect that codecov might have changed something about their upload API. I tried updating the action and now the error is different:

error - 2024-11-14 20:11:33,140 -- Upload failed: {"message":"Token required because branch is protected"}

Maybe the problem is that the branch is on my fork and not on the main repository?

Changes addressed

[saeub]

Looks like upgrading and downgrading the codecov action did the trick 😅