Repository: aeye-lab/pymovements
Pull request #224 (2023-03-01 16:11:52)

Tests: 100% coverage for transforms.py
## Description

I added the missing test cases for transforms.py. In addition, I included how to handle one more case in norm(), see below under new functionality.

Fixes issue #180 

## Implemented changes

I wrote the test as specified here:

- [x] 100% test coverage of transforms.py
  - [x] pix2deg()
    - [x] test 4 channel input
    - [x] test 4 channel input with wrong screen dimensions
    - [x] test invalid origin string
  - [x] pos2vel()
    - [x] wrong dimensions for arr
    - [x] kwargs passed but not savitzky golay
    - [x] savitzky golay two channel arr
  - [x] norm()
    - [x] arr.ndim == 3 and axis == None
    - [x] arr.ndim == 4 and axis == None
    - [x] arr.ndim == 4 and axis == 2
    
New functionality:
- [x] add additional raise of ValueError in norm() if `axis == None` and `arr.ndim > 3`

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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

## 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/datasets/dataset.py b/src/pymovements/datasets/dataset.py
index d3da437..5c91ed3 100644
--- a/src/pymovements/datasets/dataset.py
+++ b/src/pymovements/datasets/dataset.py
@@ -311,7 +311,8 @@ class Dataset:
         gaze_dfs: list[pl.DataFrame] = []
 
         # Read gaze files from fileinfo attribute.
-        for file_id, filepath in enumerate(tqdm(self.fileinfo['filepath'])):
+        for fileinfo in tqdm(self.fileinfo.to_dicts()):
+            filepath = Path(fileinfo['filepath'])
             filepath = self.raw_rootpath / filepath
 
             if preprocessed:
@@ -327,21 +328,7 @@ class Dataset:
                 raise RuntimeError(f'data files of type {filepath.suffix} are not supported')
 
             # Add fileinfo columns to dataframe.
-            for column in self.fileinfo.columns[::-1]:
-                if column == 'filepath':
-                    continue
-
-                column_value = self.fileinfo.select(column)[file_id][0, 0]
-                gaze_df = gaze_df.select([
-                    pl.lit(column_value).alias(column),
-                    pl.all(),
-                ])
-
-            # Cast columns from fileinfo according to specification.
-            gaze_df = gaze_df.with_columns([
-                pl.col(fileinfo_key).cast(fileinfo_dtype)
-                for fileinfo_key, fileinfo_dtype in self._filename_regex_dtypes.items()
-            ])
+            gaze_df = self._add_fileinfo(gaze_df, fileinfo)
 
             gaze_dfs.append(gaze_df)
 
@@ -370,19 +357,22 @@ class Dataset:
         """
         self._check_fileinfo()
 
-        file_dfs: list[pl.DataFrame] = []
+        event_dfs: list[pl.DataFrame] = []
 
         # read and preprocess input files
-        for _, filepath in enumerate(tqdm(self.fileinfo['filepath'])):
-            filepath = self.raw_rootpath / Path(filepath)
+        for fileinfo in tqdm(self.fileinfo.to_dicts()):
+            filepath = Path(fileinfo['filepath'])
+            filepath = self.raw_rootpath / filepath
 
             filepath = self._raw_to_event_filepath(filepath, events_dirname=events_dirname)
+            event_df = pl.read_ipc(filepath)
 
-            file_df = pl.read_ipc(filepath)
+            # Add fileinfo columns to dataframe.
+            event_df = self._add_fileinfo(event_df, fileinfo)
 
-            file_dfs.append(file_df)
+            event_dfs.append(event_df)
 
-        return file_dfs
+        return event_dfs
 
     def pix2deg(self, verbose: bool = True) -> None:
         """Compute gaze positions in degrees of visual angle from pixel coordinates.
@@ -520,13 +510,17 @@ class Dataset:
 
         event_dfs: list[pl.DataFrame] = []
 
-        for gaze_df in tqdm(self.gaze, disable=disable_progressbar):
+        for gaze_df, fileinfo in tqdm(
+                zip(self.gaze, self.fileinfo.to_dicts()), disable=disable_progressbar,
+        ):
 
             positions = gaze_df.select(position_columns).to_numpy()
             velocities = gaze_df.select(velocity_columns).to_numpy()
 
-            events = method(positions=positions, velocities=velocities, **kwargs)
-            event_dfs.append(events)
+            event_df = method(positions=positions, velocities=velocities, **kwargs)
+            event_df = self._add_fileinfo(event_df, fileinfo)
+
+            event_dfs.append(event_df)
 
         if not self.events or clear:
             self.events = event_dfs
@@ -860,3 +854,34 @@ class Dataset:
         events_filename = raw_filepath.stem + '.feather'
 
         return events_file_dirpath / events_filename
+
+    def _add_fileinfo(self, df: pl.DataFrame, fileinfo: dict[str, Any]) -> pl.DataFrame:
+        """Add columns from fileinfo to dataframe.
+
+        Parameters
+        ----------
+        df : pl.DataFrame
+            Base dataframe to add fileinfo to.
+        fileinfo : dict[str, Any]
+            Dictionary of fileinfo row.
+
+        Returns
+        -------
+        pl.DataFrame:
+            Dataframe with added columns from fileinfo dictionary keys.
+        """
+
+        df = df.select(
+            [
+                pl.lit(value).alias(column)
+                for column, value in fileinfo.items()
+                if column != 'filepath'
+            ] + [pl.all()],
+        )
+
+        # Cast columns from fileinfo according to specification.
+        df = df.with_columns([
+            pl.col(fileinfo_key).cast(fileinfo_dtype)
+            for fileinfo_key, fileinfo_dtype in self._filename_regex_dtypes.items()
+        ])
+        return df
diff --git a/src/pymovements/transforms.py b/src/pymovements/transforms.py
index f9c5e49..91b6f5e 100644
--- a/src/pymovements/transforms.py
+++ b/src/pymovements/transforms.py
@@ -87,6 +87,22 @@ def pix2deg(
     ...    origin='center',
     ... )
     array([[ 3.07379946, 20.43909054]])
+
+    If the eye-movement of both eyes was recorded, the input array must contain
+    first the x- and y-coordinates of one eye and then the x- and y-coordinates
+    of the other eye as shown in the example below.
+
+    >>> x_left, y_left = 123.0, 865.0
+    >>> x_right, y_right = 183.0, 865.0
+    >>> arr = [(x_left, y_left, x_right, y_right)]
+    >>> pix2deg(
+    ...    arr=arr,
+    ...    screen_px=(1280, 1024),
+    ...    screen_cm=(38.0, 30.0),
+    ...    distance_cm=68.0,
+    ...    origin='center',
+    ... )
+    array([[ 3.07379946, 20.43909054,  4.56790364, 20.43909054]])
     """
     if arr is None:
         raise TypeError('arr must not be None')
@@ -291,6 +307,12 @@ def norm(arr: np.ndarray, axis: int | None = None) -> np.ndarray | Any:
     -------
     np.ndarray
 
+    Raises
+    ------
+    ValueError
+        If no axis is given but the array dimensions are lager than 3.
+        In that case the axis cannot be inferred.
+
     Examples
     --------
     >>> arr = np.array([[1., 1., 1., 1., 1., 1.], [1., 1., 1., 1., 1., 1.]])
@@ -309,6 +331,14 @@ def norm(arr: np.ndarray, axis: int | None = None) -> np.ndarray | Any:
         elif arr.ndim == 3:
             axis = 1
 
+        else:
+            raise ValueError(
+                f'Axis can not be inferred in case of more than 3 '
+                f'input array dimensions (arr.shape={arr.shape}).'
+                f' Either reduce the number of input array '
+                f'dimensions or specify `axis` explicitly.',
+            )
+
     return np.linalg.norm(arr, axis=axis)
 
 


Test patch:
diff --git a/tests/datasets/test_dataset.py b/tests/datasets/test_dataset.py
index e964177..2898d9d 100644
--- a/tests/datasets/test_dataset.py
+++ b/tests/datasets/test_dataset.py
@@ -157,11 +157,12 @@ def mock_toy(rootpath, raw_fileformat='csv'):
     for fileinfo_row in fileinfo.to_dicts():
         event_df = pl.from_dict(
             {
+                'subject_id': fileinfo_row['subject_id'],
                 'type': 'saccade',
                 'onset': np.arange(0, 901, 100),
                 'offset': np.arange(10, 911, 100),
             },
-            schema={'type': pl.Utf8, 'onset': pl.Int64, 'offset': pl.Int64},
+            schema={'subject_id': pl.Int64, 'type': pl.Utf8, 'onset': pl.Int64, 'offset': pl.Int64},
         )
         event_dfs.append(event_df)
 
@@ -417,8 +418,9 @@ def test_detect_events(detect_event_kwargs, dataset_configuration):
     dataset.pos2vel()
     dataset.detect_events(**detect_event_kwargs)
 
+    expected_schema = {'subject_id': pl.Int64, **Saccade.schema}
     for result_event_df in dataset.events:
-        assert result_event_df.schema == Saccade.schema
+        assert result_event_df.schema == expected_schema
 
 
 @pytest.mark.parametrize(
@@ -435,7 +437,7 @@ def test_detect_events(detect_event_kwargs, dataset_configuration):
                 'threshold': 1,
                 'eye': 'auto',
             },
-            Saccade.schema,
+            {'subject_id': pl.Int64, **Saccade.schema},
             id='two-saccade-runs',
         ),
         pytest.param(
@@ -449,7 +451,7 @@ def test_detect_events(detect_event_kwargs, dataset_configuration):
                 'velocity_threshold': 1,
                 'minimum_duration': 1,
             },
-            {**Saccade.schema, **Fixation.schema},
+            {'subject_id': pl.Int64, **Saccade.schema, **Fixation.schema},
             id='one-saccade-one-fixation-run',
         ),
     ],
diff --git a/tests/test_transforms.py b/tests/test_transforms.py
index abe0f17..67af996 100644
--- a/tests/test_transforms.py
+++ b/tests/test_transforms.py
@@ -23,11 +23,11 @@ Test all functions in pymovements.transforms.
 import numpy as np
 import pytest
 
+from pymovements.transforms import norm
 from pymovements.transforms import pix2deg
 from pymovements.transforms import pos2vel
 from pymovements.transforms import split
 
-
 n_coords = 100
 screen_px_1d = 100
 screen_cm_1d = 100
@@ -181,6 +181,39 @@ screen_cm_2d = [100, 100]
             ValueError,
             id='list_coords_3d_raises_value_error',
         ),
+        pytest.param(
+            {
+                'arr': np.zeros((n_coords, 4)),
+                'screen_px': screen_px_1d,
+                'screen_cm': screen_px_2d,
+                'distance_cm': screen_cm_1d,
+                'origin': 'center',
+            },
+            ValueError,
+            id='list_coords_4d_screen_px_not_2d_raises_value_error',
+        ),
+        pytest.param(
+            {
+                'arr': np.zeros((n_coords, 4)),
+                'screen_px': screen_px_2d,
+                'screen_cm': screen_cm_1d,
+                'distance_cm': screen_cm_1d,
+                'origin': 'center',
+            },
+            ValueError,
+            id='list_coords_4d_screen_cm_not_2d_raises_value_error',
+        ),
+        pytest.param(
+            {
+                'arr': np.zeros((n_coords, 4)),
+                'screen_px': screen_px_2d,
+                'screen_cm': screen_px_2d,
+                'distance_cm': screen_cm_1d,
+                'origin': 'invalid',
+            },
+            ValueError,
+            id='invalid_origin_str_raises_value_error',
+        ),
     ],
 )
 def test_pix2deg_raises_error(kwargs, expected_error):
@@ -367,6 +400,17 @@ def test_pix2deg_raises_error(kwargs, expected_error):
             np.array([[0.0, 0.0]] * n_coords),
             id='nparray_of_zero_coords_2d',
         ),
+        pytest.param(
+            {
+                'arr': np.array([[0.0, 0.0, 0.0, 0.0]] * n_coords),
+                'screen_px': screen_px_2d,
+                'screen_cm': screen_cm_2d,
+                'distance_cm': screen_cm_1d,
+                'origin': 'center',
+            },
+            np.array([[0.0, 0.0, 0.0, 0.0]] * n_coords),
+            id='nparray_of_zero_coords_4d',
+        ),
     ],
 )
 def test_pix2deg_returns(kwargs, expected_value):
@@ -425,6 +469,23 @@ def test_pix2deg_returns(kwargs, expected_value):
             ValueError,
             id='invalid_method_raises_value_error',
         ),
+        pytest.param(
+            {
+                'arr': np.ones((3, 3, 3)),
+                'method': 'smooth',
+            },
+            ValueError,
+            id='wrong_dimensions_input_arr_raises_value_error',
+        ),
+        pytest.param(
+            {
+                'arr': np.ones(10),
+                'method': 'smooth',
+                'kwargs': {},
+            },
+            ValueError,
+            id='kwargs_passed_but_method_not_savitzky_golay_raises_value_error',
+        ),
     ],
 )
 def test_pos2vel_raises_error(kwargs, expected_error):
@@ -508,6 +569,92 @@ def test_pos2vel_stepped_input_returns(params, expected_value):
     assert np.allclose(actual_value[lpad:rpad], expected_value[lpad:rpad])
 
 
+@pytest.mark.parametrize(
+    'params, expected_value',
+    [
+        pytest.param(
+            {'method': 'savitzky_golay', 'window_length': 7, 'polyorder': 2, 'sampling_rate': 1},
+            np.concatenate([
+                np.array([
+                    [0.71428571, 0.71428571],
+                    [0.80952381, 0.80952381],
+                    [0.9047619, 0.9047619],
+                ]),
+                np.ones((94, 2)),
+                np.array([
+                    [0.9047619, 0.9047619],
+                    [0.80952381, 0.80952381],
+                    [0.71428571, 0.71428571],
+                ]),
+            ]),
+            id='method_savitzky_golay_linear_velocity_2d_input',
+        ),
+    ],
+)
+def test_pos2vel_2d_stepped_input_returns(params, expected_value):
+    N = 100
+    x = np.linspace(0, N - 2, N // 2)
+    x = np.repeat(x, 4)
+    x = np.reshape(x, (100, 2))
+
+    actual_value = pos2vel(x, **params)
+
+    lpad, rpad = 1, -1
+    assert np.allclose(actual_value[lpad:rpad], expected_value[lpad:rpad])
+
+
+@pytest.mark.parametrize(
+    'params, expected_value',
+    [
+        pytest.param(
+            {'arr': np.ones((2, 2, 5)), 'axis': None},
+            np.array([
+                [1.41421356, 1.41421356, 1.41421356, 1.41421356, 1.41421356],
+                [1.41421356, 1.41421356, 1.41421356, 1.41421356, 1.41421356],
+            ]),
+            id='3_dim_array_no_axis',
+        ),
+        pytest.param(
+            {'arr': np.ones((2, 5)), 'axis': None},
+            np.array([[1.41421356, 1.41421356, 1.41421356, 1.41421356, 1.41421356]]),
+            id='2_dim_array_no_axis',
+        ),
+        pytest.param(
+            {'arr': np.ones((2, 2, 2, 2)), 'axis': 2},
+            np.array([
+                [
+                    [1.41421356, 1.41421356],
+                    [1.41421356, 1.41421356],
+                ],
+                [
+                    [1.41421356, 1.41421356],
+                    [1.41421356, 1.41421356],
+                ],
+            ]),
+            id='4_dim_array_with_axis',
+        ),
+    ],
+)
+def test_norm(params, expected_value):
+    actual_value = norm(**params)
+    assert np.allclose(actual_value, expected_value)
+
+
+@pytest.mark.parametrize(
+    'params, expected_error',
+    [
+        pytest.param(
+            {'arr': np.ones((2, 2, 5, 5)), 'axis': None},
+            ValueError,
+            id='4_dim_array_no_axis_raises_value_error',
+        ),
+    ],
+)
+def test_norm_raises_exception(params, expected_error):
+    with pytest.raises(expected_error):
+        norm(**params)
+
+
 @pytest.mark.parametrize(
     'params, expected',
     [