Repository: aeye-lab/pymovements
Pull request #1085 (2025-04-09 11:40:20)

chore: refactor filling experiment in from_asc()
## Description

Refactors the filling in of experiment data from parsed metadata in `from_asc()`.

Required by #1004 to reduce number of branches (pylint error).

## Implemented changes

- [x] create `_fill_experiment_from_parsing_metadata()` function
- [x] change signifier in error messages from `vs.` to `!=`

## How Has This Been Tested?

This is a pure refactor (except for the changed error messages).
No additional tests needed. All previous tests pass.

## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream modules
- [x] I have checked my code and corrected any misspellings


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/aeye-lab/pymovements/pull/1085?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 99.94%. Comparing base [(`e7bae90`)](https://app.codecov.io/gh/aeye-lab/pymovements/commit/e7bae90c734dd3d1897cd12a6242c7bc7971cc96?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) to head [(`0b22d0f`)](https://app.codecov.io/gh/aeye-lab/pymovements/commit/0b22d0f1c221f48be64755eeed30f0484d98f7c2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##              main    #1085      +/-   ##
===========================================
- Coverage   100.00%   99.94%   -0.06%     
===========================================
  Files           87       87              
  Lines         3717     3720       +3     
  Branches       638      638              
===========================================
+ Hits          3717     3718       +1     
- Misses           0        1       +1     
- Partials         0        1       +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/aeye-lab/pymovements/pull/1085?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/src/pymovements/gaze/io.py b/src/pymovements/gaze/io.py
index ea92d93..e6f73da 100644
--- a/src/pymovements/gaze/io.py
+++ b/src/pymovements/gaze/io.py
@@ -377,70 +377,8 @@ def from_asc(
             for fileinfo_key, fileinfo_dtype in column_schema_overrides.items()
         ])
 
-    if experiment is None:
-        experiment = Experiment(sampling_rate=metadata['sampling_rate'])
-
-    # Compare metadata from experiment definition with metadata from ASC file.
-    # Fill in missing metadata in experiment definition and raise an error if there are conflicts
-    issues = []
-
-    # Screen resolution (assuming that width and height will always be missing or set together)
-    experiment_resolution = (experiment.screen.width_px, experiment.screen.height_px)
-    if experiment_resolution == (None, None):
-        experiment.screen.width_px, experiment.screen.height_px = metadata['resolution']
-    elif experiment_resolution != metadata['resolution']:
-        issues.append(f"Screen resolution: {experiment_resolution} vs. {metadata['resolution']}")
-
-    # Sampling rate
-    if experiment.eyetracker.sampling_rate != metadata['sampling_rate']:
-        issues.append(
-            f"Sampling rate: {experiment.eyetracker.sampling_rate} vs. {metadata['sampling_rate']}",
-        )
-
-    # Tracked eye
-    asc_left_eye = 'L' in (metadata['tracked_eye'] or '')
-    asc_right_eye = 'R' in (metadata['tracked_eye'] or '')
-    if experiment.eyetracker.left is None:
-        experiment.eyetracker.left = asc_left_eye
-    elif experiment.eyetracker.left != asc_left_eye:
-        issues.append(f"Left eye tracked: {experiment.eyetracker.left} vs. {asc_left_eye}")
-    if experiment.eyetracker.right is None:
-        experiment.eyetracker.right = asc_right_eye
-    elif experiment.eyetracker.right != asc_right_eye:
-        issues.append(f"Right eye tracked: {experiment.eyetracker.right} vs. {asc_right_eye}")
-
-    # Mount configuration
-    if experiment.eyetracker.mount is None:
-        experiment.eyetracker.mount = metadata['mount_configuration']['mount_type']
-    elif experiment.eyetracker.mount != metadata['mount_configuration']['mount_type']:
-        issues.append(f"Mount configuration: {experiment.eyetracker.mount} vs. "
-                      f"{metadata['mount_configuration']['mount_type']}")
-
-    # Eye tracker vendor
-    asc_vendor = 'EyeLink' if 'EyeLink' in metadata['model'] else None
-    if experiment.eyetracker.vendor is None:
-        experiment.eyetracker.vendor = asc_vendor
-    elif experiment.eyetracker.vendor != asc_vendor:
-        issues.append(f"Eye tracker vendor: {experiment.eyetracker.vendor} vs. {asc_vendor}")
-
-    # Eye tracker model
-    if experiment.eyetracker.model is None:
-        experiment.eyetracker.model = metadata['model']
-    elif experiment.eyetracker.model != metadata['model']:
-        issues.append(f"Eye tracker model: {experiment.eyetracker.model} vs. {metadata['model']}")
-
-    # Eye tracker software version
-    if experiment.eyetracker.version is None:
-        experiment.eyetracker.version = metadata['version_number']
-    elif experiment.eyetracker.version != metadata['version_number']:
-        issues.append(f"Eye tracker software version: {experiment.eyetracker.version} vs. "
-                      f"{metadata['version_number']}")
-
-    if issues:
-        raise ValueError(
-            'Experiment metadata does not match the metadata in the ASC file:\n'
-            + '\n'.join(f'- {issue}' for issue in issues),
-        )
+    # Fill experiment with parsed metadata.
+    experiment = _fill_experiment_from_parsing_metadata(experiment, metadata)
 
     # Create gaze data frame.
     gaze_df = GazeDataFrame(
@@ -555,3 +493,76 @@ def from_ipc(
         trial_columns=trial_columns,
     )
     return gaze_df
+
+
+def _fill_experiment_from_parsing_metadata(
+        experiment: Experiment | None,
+        metadata: dict[str, Any],
+) -> Experiment:
+    """Fill Experiment with metadata gained from parsing."""
+    if experiment is None:
+        experiment = Experiment(sampling_rate=metadata['sampling_rate'])
+
+    # Compare metadata from experiment definition with metadata from ASC file.
+    # Fill in missing metadata in experiment definition and raise an error if there are conflicts
+    issues = []
+
+    # Screen resolution (assuming that width and height will always be missing or set together)
+    experiment_resolution = (experiment.screen.width_px, experiment.screen.height_px)
+    if experiment_resolution == (None, None):
+        experiment.screen.width_px, experiment.screen.height_px = metadata['resolution']
+    elif experiment_resolution != metadata['resolution']:
+        issues.append(f"Screen resolution: {experiment_resolution} != {metadata['resolution']}")
+
+    # Sampling rate
+    if experiment.eyetracker.sampling_rate != metadata['sampling_rate']:
+        issues.append(
+            f"Sampling rate: {experiment.eyetracker.sampling_rate} != {metadata['sampling_rate']}",
+        )
+
+    # Tracked eye
+    asc_left_eye = 'L' in (metadata['tracked_eye'] or '')
+    asc_right_eye = 'R' in (metadata['tracked_eye'] or '')
+    if experiment.eyetracker.left is None:
+        experiment.eyetracker.left = asc_left_eye
+    elif experiment.eyetracker.left != asc_left_eye:
+        issues.append(f"Left eye tracked: {experiment.eyetracker.left} != {asc_left_eye}")
+    if experiment.eyetracker.right is None:
+        experiment.eyetracker.right = asc_right_eye
+    elif experiment.eyetracker.right != asc_right_eye:
+        issues.append(f"Right eye tracked: {experiment.eyetracker.right} != {asc_right_eye}")
+
+    # Mount configuration
+    if experiment.eyetracker.mount is None:
+        experiment.eyetracker.mount = metadata['mount_configuration']['mount_type']
+    elif experiment.eyetracker.mount != metadata['mount_configuration']['mount_type']:
+        issues.append(f"Mount configuration: {experiment.eyetracker.mount} != "
+                      f"{metadata['mount_configuration']['mount_type']}")
+
+    # Eye tracker vendor
+    asc_vendor = 'EyeLink' if 'EyeLink' in metadata['model'] else None
+    if experiment.eyetracker.vendor is None:
+        experiment.eyetracker.vendor = asc_vendor
+    elif experiment.eyetracker.vendor != asc_vendor:
+        issues.append(f"Eye tracker vendor: {experiment.eyetracker.vendor} != {asc_vendor}")
+
+    # Eye tracker model
+    if experiment.eyetracker.model is None:
+        experiment.eyetracker.model = metadata['model']
+    elif experiment.eyetracker.model != metadata['model']:
+        issues.append(f"Eye tracker model: {experiment.eyetracker.model} != {metadata['model']}")
+
+    # Eye tracker software version
+    if experiment.eyetracker.version is None:
+        experiment.eyetracker.version = metadata['version_number']
+    elif experiment.eyetracker.version != metadata['version_number']:
+        issues.append(f"Eye tracker software version: {experiment.eyetracker.version} != "
+                      f"{metadata['version_number']}")
+
+    if issues:
+        raise ValueError(
+            'Experiment metadata does not match the metadata in the ASC file:\n'
+            + '\n'.join(f'- {issue}' for issue in issues),
+        )
+
+    return experiment


Test patch:
diff --git a/tests/unit/gaze/io/asc_test.py b/tests/unit/gaze/io/asc_test.py
index 1e0272d..1439f54 100644
--- a/tests/unit/gaze/io/asc_test.py
+++ b/tests/unit/gaze/io/asc_test.py
@@ -323,14 +323,14 @@ def test_from_asc_fills_in_experiment_metadata(file, metadata):
                 'screen_height_px': 1080,
                 'sampling_rate': 1000,
             },
-            ['Screen resolution: (1920, 1080) vs. (1280, 1024)'],
+            ['Screen resolution: (1920, 1080) != (1280, 1024)'],
             id='screen_resolution',
         ),
         pytest.param(
             {
                 'eyetracker': pm.EyeTracker(sampling_rate=500),
             },
-            ['Sampling rate: 500 vs. 1000.0'],
+            ['Sampling rate: 500 != 1000.0'],
             id='eyetracker_sampling_rate',
         ),
         pytest.param(
@@ -343,8 +343,8 @@ def test_from_asc_fills_in_experiment_metadata(file, metadata):
                 ),
             },
             [
-                'Left eye tracked: False vs. True',
-                'Right eye tracked: True vs. False',
+                'Left eye tracked: False != True',
+                'Right eye tracked: True != False',
             ],
             id='eyetracker_tracked_eye',
         ),
@@ -360,9 +360,9 @@ def test_from_asc_fills_in_experiment_metadata(file, metadata):
                 ),
             },
             [
-                'Eye tracker vendor: Tobii vs. EyeLink',
-                'Eye tracker model: Tobii Pro Spectrum vs. EyeLink Portable Duo',
-                'Eye tracker software version: 1.0 vs. 6.12',
+                'Eye tracker vendor: Tobii != EyeLink',
+                'Eye tracker model: Tobii Pro Spectrum != EyeLink Portable Duo',
+                'Eye tracker software version: 1.0 != 6.12',
             ],
             id='eyetracker_vendor_model_version',
         ),
@@ -376,7 +376,7 @@ def test_from_asc_fills_in_experiment_metadata(file, metadata):
                     version='6.12',
                 ),
             },
-            ['Mount configuration: Remote vs. Desktop'],
+            ['Mount configuration: Remote != Desktop'],
             id='eyetracker_mount',
         ),
     ],