Repository: aeye-lab/pymovements (python)

Issue:
Use GAZE_COORDS instead of DISPLAY_COORDS for resolution detection
## Current Behavior

`parse_eyelink()` currently uses the `DISPLAY_COORDS` message to detect the screen resolution. This is an optional message to define the resolution for Data Viewer, and some ASC files may not contain such a message. `from_asc()` results in an error in such cases.

## Expected Behavior

The `GAZE_COORDS` should be used because (as far as I can tell) it is always logged at the beginning of a trial. Theoretically, it might be possible to use a different resolution for each trial, but I don't expect this to be common, so we could just proceed in the same way as with the sampling rate (#887), where we warn the user about inconsistent resolution settings.

## Minimum acceptance criteria

- ASC files without `DISPLAY_COORDS` message parsed correctly

Pull request:
fix: Use GAZE_COORDS message to determine screen resolution
## Description

Follow-up on #887. Screen resolution should be parsed from the `GAZE_COORDS` messages instead of the optional `DIAPLAY_COORDS` message. `GAZE_COORDS` is logged before the start of a new recording block, after `RECCFG`, so adding it to the `recording_config` metadata makes sense. This also checks and warns the user if the resolution has been changed throughout the session. The `resolution` key in the metadata dictionary is kept, so no breaking change.

## Implemented changes

- [x] Use `GAZE_COORDS` message instead of `DISPLAY_COORDS` message to parse screen resolution
- [x] Include screen resolution in `recording_config`, because it can change every time recording is started
- [x] Check for consistent screen resolution, analogous to #887

## How Has This Been Tested?

- [x] Added test case for inconsistent screen resolution
- [x] Tested with a real ASC files that doesn't have a `DISPLAY_COORDS` message

## Type of change

- Bug fix

## Context

Resolves #1086

If #945 is merged first, pylint's complaint about "too many statements" will disappear. (`parse_eyelink()` is quite bloated at the moment, but #945 will make it slimmer.)

## 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


Patch:
diff --git a/src/pymovements/gaze/_utils/parsing.py b/src/pymovements/gaze/_utils/parsing.py
index 4afbb45..009347b 100755
--- a/src/pymovements/gaze/_utils/parsing.py
+++ b/src/pymovements/gaze/_utils/parsing.py
@@ -18,6 +18,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 """Module for parsing input data."""
+# pylint: disable=too-many-statements
 from __future__ import annotations
 
 import calendar
@@ -48,7 +49,7 @@ EYELINK_META_REGEXES = [
             r'\s+(?P<day>\d\d?)\s+(?P<time>\d\d:\d\d:\d\d)\s+(?P<year>\d{4})\s*'
         ),
         r'\*\*\s+(?P<version_2>EYELINK.*)',
-        r'MSG\s+\d+[.]?\d*\s+DISPLAY_COORDS\s*=?\s*(?P<resolution>.*)',
+        r'MSG\s+\d+[.]?\d*\s+DISPLAY_COORDS\s*=?\s*(?P<DISPLAY_COORDS>.*)',
         r'PUPIL\s+(?P<pupil_data_type>(AREA|DIAMETER))\s*',
         r'MSG\s+\d+[.]?\d*\s+ELCLCFG\s+(?P<mount_configuration>.*)',
     )
@@ -95,6 +96,9 @@ RECORDING_CONFIG = re.compile(
     r'(?P<link_sample_filter>0|1|2)\s+'
     r'(?P<tracked_eye>LR|[LR])\s*',
 )
+RESOLUTION_REGEX = re.compile(
+    r'MSG\s+\d+[.]?\d*\s+GAZE_COORDS\s*=?\s*(?P<resolution>.*)',
+)
 
 
 def check_nan(sample_location: str) -> float:
@@ -303,6 +307,11 @@ def parse_eyelink(
         elif eye_side_match := RECORDING_CONFIG.match(line):
             recording_config.append(eye_side_match.groupdict())
 
+        elif match := RESOLUTION_REGEX.match(line):
+            left, top, right, bottom = (float(coord) for coord in match.group('resolution').split())
+            # GAZE_COORDS is always logged after RECCFG -> add it to the last recording_config
+            recording_config[-1]['resolution'] = (right - left + 1, bottom - top + 1)
+
         elif match := START_RECORDING_REGEX.match(line):
             start_recording_timestamp = match.groupdict()['timestamp']
 
@@ -373,6 +382,7 @@ def parse_eyelink(
     )
 
     metadata['tracked_eye'] = _check_reccfg_key(recording_config, 'tracked_eye')
+    metadata['resolution'] = _check_reccfg_key(recording_config, 'resolution')
 
     pre_processed_metadata: dict[str, Any] = _pre_process_metadata(metadata)
     # is not yet pre-processed but should be
@@ -416,10 +426,9 @@ def _pre_process_metadata(metadata: defaultdict[str, Any]) -> dict[str, Any]:
         metadata['version_1'], metadata['version_2'],
     )
 
-    if 'resolution' in metadata:
-        coordinates = [int(coord) for coord in metadata['resolution'].split()]
-        resolution = (coordinates[2] - coordinates[0] + 1, coordinates[3] - coordinates[1] + 1)
-        metadata['resolution'] = resolution
+    if 'DISPLAY_COORDS' in metadata:
+        display_coords = tuple(float(coord) for coord in metadata['DISPLAY_COORDS'].split())
+        metadata['DISPLAY_COORDS'] = display_coords
 
     # if the date has been parsed fully, convert the date to a datetime object
     if 'day' in metadata and 'year' in metadata and 'month' in metadata and 'time' in metadata:
diff --git a/src/pymovements/gaze/io.py b/src/pymovements/gaze/io.py
index e6f73da..db42c7f 100644
--- a/src/pymovements/gaze/io.py
+++ b/src/pymovements/gaze/io.py
@@ -20,6 +20,7 @@
 """Functionality to load GazeDataFrame from a csv file."""
 from __future__ import annotations
 
+import math
 from pathlib import Path
 from typing import Any
 
@@ -510,7 +511,9 @@ def _fill_experiment_from_parsing_metadata(
     # 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']
+        width, height = metadata['resolution']
+        experiment.screen.width_px = math.ceil(width)
+        experiment.screen.height_px = math.ceil(height)
     elif experiment_resolution != metadata['resolution']:
         issues.append(f"Screen resolution: {experiment_resolution} != {metadata['resolution']}")
 


Test patch:
diff --git a/tests/unit/gaze/_utils/_parsing_test.py b/tests/unit/gaze/_utils/_parsing_test.py
index 892c114..0aebf71 100644
--- a/tests/unit/gaze/_utils/_parsing_test.py
+++ b/tests/unit/gaze/_utils/_parsing_test.py
@@ -43,9 +43,10 @@ ASC_TEXT = r"""
 
 some
 lines
+MSG	2095865 DISPLAY_COORDS 0 0 1279 1023
 MSG	2154555 RECCFG CR 1000 2 1 L
 MSG	2154555 ELCLCFG BTABLER
-MSG	2095865 DISPLAY_COORDS 0 0 1279 1023
+MSG	2154555 GAZE_COORDS 0.00 0.00 1279.00 1023.00
 PRESCALER	1
 VPRESCALER	1
 PUPIL	AREA
@@ -166,6 +167,7 @@ EXPECTED_METADATA = {
     'calibrations': [],
     'validations': [],
     'resolution': (1280, 1024),
+    'DISPLAY_COORDS': (0.0, 0.0, 1279.0, 1023.0),
     'data_loss_ratio_blinks': 0.18181818181818182,
     'data_loss_ratio': 0.2727272727272727,
     'total_recording_duration_ms': 11.0,
@@ -194,6 +196,7 @@ EXPECTED_METADATA = {
             'timestamp': '2154555',
             'tracked_eye': 'L',
             'tracking_mode': 'CR',
+            'resolution': (1280.0, 1024.0),
         },
     ],
 }
@@ -369,7 +372,16 @@ def test_parse_eyelink_version(tmp_path, metadata, expected_version, expected_mo
             'MSG	2154555 RECCFG CR 1000 2 1 L\n'
             'MSG	2154556 RECCFG CR 2000 2 1 L\n',
             r"Found inconsistent values for 'sampling_rate': \['1000', '2000'\]",
-            id='inconsistent_reccfg',
+            id='inconsistent_sampling_rate',
+        ),
+        pytest.param(
+            'MSG	2154555 RECCFG CR 1000 2 1 L\n'
+            'MSG	2154555 GAZE_COORDS 0 0 1919 1079\n'
+            'MSG	2154556 RECCFG CR 1000 2 1 L\n'
+            'MSG	2154556 GAZE_COORDS 0 0 1023 767\n',
+            "Found inconsistent values for 'resolution': "
+            r'\[\(1024.0, 768.0\), \(1920.0, 1080.0\)\]',
+            id='inconsistent_resolution',
         ),
     ],
 )
@@ -389,7 +401,6 @@ def test_metadata_warnings(tmp_path, metadata, expected_msg):
     ('metadata', 'expected_validation', 'expected_calibration'),
     [
         pytest.param(
-            'MSG	2095865 DISPLAY_COORDS 0 0 1279 1023\n'
             'MSG	7045618 !CAL \n'
             '>>>>>>> CALIBRATION (HV9,P-CR) FOR LEFT: <<<<<<<<<\n'
             'MSG	7045618 !CAL Calibration points:  \n'
diff --git a/tests/unit/gaze/io/asc_test.py b/tests/unit/gaze/io/asc_test.py
index 1439f54..2f8482e 100644
--- a/tests/unit/gaze/io/asc_test.py
+++ b/tests/unit/gaze/io/asc_test.py
@@ -323,7 +323,7 @@ def test_from_asc_fills_in_experiment_metadata(file, metadata):
                 'screen_height_px': 1080,
                 'sampling_rate': 1000,
             },
-            ['Screen resolution: (1920, 1080) != (1280, 1024)'],
+            ['Screen resolution: (1920, 1080) != (1280.0, 1024.0)'],
             id='screen_resolution',
         ),
         pytest.param(