Repository: aeye-lab/pymovements
Pull request #172 (2023-02-20 17:02:31)

Transforms: Refactor cut_into_subsequences to split
## Description

Fixes issue #144 

We now take use of the numpy function  [`np.split()`](https://numpy.org/doc/stable/reference/generated/numpy.split.html)

Moreover, I really despise the current name of the function as its very long and verbose.
I propose to rename the function just simply to `split()`.

## Implemented changes

- [x] use np.split() instead of inner for loop

## Type of change

- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)

## How Has This Been Tested?

- [x] Additional test for a batch of two sequences that need to be padded
- [x] Additional test for different value of window_size

## 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-commenter: # [Codecov](https://codecov.io/gh/aeye-lab/pymovements/pull/172?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) Report
Base: **59.87**% // Head: **59.71**% // Decreases project coverage by **`-0.16%`** :warning:
> Coverage data is based on head [(`47f953a`)](https://codecov.io/gh/aeye-lab/pymovements/pull/172?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) compared to base [(`2b2f909`)](https://codecov.io/gh/aeye-lab/pymovements/commit/2b2f90975d81daf15cb123aedb65504f38f5c123?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).
> Patch coverage: 100.00% of modified lines in pull request are covered.

> :exclamation: Current head 47f953a differs from pull request most recent head 93c1f94. Consider uploading reports for the commit 93c1f94 to get more accurate results

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


```diff
@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
- Coverage   59.87%   59.71%   -0.16%     
==========================================
  Files          21       21              
  Lines         775      772       -3     
  Branches      178      177       -1     
==========================================
- Hits          464      461       -3     
  Misses        299      299              
  Partials       12       12              
```


| [Impacted Files](https://codecov.io/gh/aeye-lab/pymovements/pull/172?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab) | Coverage Δ | |
|---|---|---|
| [src/pymovements/transforms.py](https://codecov.io/gh/aeye-lab/pymovements/pull/172?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab#diff-c3JjL3B5bW92ZW1lbnRzL3RyYW5zZm9ybXMucHk=) | `82.07% <100.00%> (-0.50%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/aeye-lab/pymovements/pull/172?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aeye-lab).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](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).

dkrako: I don't know why test coverage is decreasing, the `split()` function is covered 100%.

Patch:
diff --git a/src/pymovements/transforms.py b/src/pymovements/transforms.py
index 551e38d..f9c5e49 100644
--- a/src/pymovements/transforms.py
+++ b/src/pymovements/transforms.py
@@ -312,10 +312,10 @@ def norm(arr: np.ndarray, axis: int | None = None) -> np.ndarray | Any:
     return np.linalg.norm(arr, axis=axis)
 
 
-def cut_into_subsequences(
+def split(
     arr: np.ndarray, window_size: int, keep_padded: bool = True,
 ) -> np.ndarray:
-    """Cut sequence into subsequences of equal length.
+    """Split sequence into subsequences of equal length.
 
     Parameters
     ----------
@@ -330,7 +330,7 @@ def cut_into_subsequences(
     Returns
     -------
     np.ndarray
-        Output sequence with new window length of shape (N', L', C)
+        Array of split sequences with new window length of shape (N', L', C)
 
     Examples
     --------
@@ -340,19 +340,19 @@ def cut_into_subsequences(
     >>> arr.shape
     (1, 13, 2)
 
-    We now cut the original array into three subsequences of length `5`.
+    We now split the original array into three subsequences of length `5`.
 
-    >>> arr_cut = cut_into_subsequences(
+    >>> arr_split = split(
     ...    arr=arr,
     ...    window_size=5,
     ...    keep_padded=True,
     ... )
-    >>> arr_cut.shape
+    >>> arr_split.shape
     (3, 5, 2)
 
     The last subsequence is padded with `nan` to have a length of `5`.
 
-    >>> arr_cut[-1]
+    >>> arr_split[-1]
     array([[ 1.,  1.],
            [ 1.,  1.],
            [ 1.,  1.],
@@ -362,12 +362,12 @@ def cut_into_subsequences(
     We can also drop any remaining sequences that would need padding by passing
     ``keep_padded=False``.
 
-    >>> arr_cut = cut_into_subsequences(
+    >>> arr_split = split(
     ...    arr=arr,
     ...    window_size=5,
     ...    keep_padded=False,
     ... )
-    >>> arr_cut.shape
+    >>> arr_split.shape
     (2, 5, 2)
 
     """
@@ -378,32 +378,26 @@ def cut_into_subsequences(
     else:
         n_rows = arr.shape[0] * n
 
-    arr_cut = np.nan * np.ones((n_rows, window_size, arr.shape[2]))
+    arr_split = np.nan * np.ones((n_rows, window_size, arr.shape[2]))
 
     idx = 0
-    for t in range(0, arr.shape[0]):
-        for i in range(0, n):
-            # get part of the sequence with window_size
-            arr_tmp = np.expand_dims(arr[t, i * window_size: (i + 1) * window_size, :], axis=0)
+    for arr_instance in arr:
+        # Create an array of indicies where sequence will be split.
+        split_indices = np.arange(start=window_size, stop=(n + 1) * window_size, step=window_size)
 
-            # fill row with respective part of the sequence
-            arr_cut[idx, :, :] = arr_tmp
+        # The length of the last element in list will be equal to rest.
+        arr_split_list = np.split(arr_instance, split_indices)
 
-            idx = idx + 1
+        # Put the first n elements of split list into output array.
+        arr_split[idx:idx + n] = arr_split_list[:-1]
+        idx += n
 
         if rest > 0 and keep_padded:
-            # get end point of last part of the sequence
-            start_idx_last_piece = window_size * n
+            # Insert last split, remaining padded samples are already np.nan.
+            arr_split[idx, :rest] = arr_split_list[-1]
+            idx += 1
 
-            # get last part of sequence which is shorter than window_size
-            arr_incomplete = arr[t, start_idx_last_piece:, :]
-
-            # insert last part, remaining samples are already np.nan
-            arr_cut[idx, :rest, :] = arr_incomplete
-
-            idx = idx + 1
-
-    return arr_cut
+    return arr_split
 
 
 def downsample(


Test patch:
diff --git a/tests/test_transforms.py b/tests/test_transforms.py
index 9286e2a..abe0f17 100644
--- a/tests/test_transforms.py
+++ b/tests/test_transforms.py
@@ -23,9 +23,9 @@ Test all functions in pymovements.transforms.
 import numpy as np
 import pytest
 
-from pymovements.transforms import cut_into_subsequences
 from pymovements.transforms import pix2deg
 from pymovements.transforms import pos2vel
+from pymovements.transforms import split
 
 
 n_coords = 100
@@ -511,6 +511,11 @@ def test_pos2vel_stepped_input_returns(params, expected_value):
 @pytest.mark.parametrize(
     'params, expected',
     [
+        pytest.param(
+            {'arr': np.ones((1, 10, 2)), 'window_size': 2, 'keep_padded': False},
+            {'value': np.ones((5, 2, 2))},
+            id='length_double_window_size2_keep_padded_returns_two_instances',
+        ),
         pytest.param(
             {'arr': np.ones((1, 10, 2)), 'window_size': 5, 'keep_padded': False},
             {'value': np.ones((2, 5, 2))},
@@ -536,15 +541,32 @@ def test_pos2vel_stepped_input_returns(params, expected_value):
             },
             id='length_double_window_size+1_not_keep_padded_returns_three_instances',
         ),
+        pytest.param(
+            {'arr': np.ones((2, 10, 2)), 'window_size': 5, 'keep_padded': False},
+            {'value': np.ones((4, 5, 2))},
+            id='two_instances_length_double_window_size_keep_padded_returns_two_instances',
+        ),
+        pytest.param(
+            {'arr': np.ones((2, 11, 2)), 'window_size': 5, 'keep_padded': True},
+            {
+                'value': np.concatenate([
+                    np.ones((2, 5, 2)),
+                    np.expand_dims(np.concatenate([np.ones((1, 2)), np.ones((4, 2)) * np.nan]), 0),
+                    np.ones((2, 5, 2)),
+                    np.expand_dims(np.concatenate([np.ones((1, 2)), np.ones((4, 2)) * np.nan]), 0),
+                ]),
+            },
+            id='two_instances_length_double_window_size+1_not_keep_padded_returns_six_instances',
+        ),
     ],
 )
 def test_cut_into_subsequences(params, expected):
     if 'exception' in expected:
         with pytest.raises(expected['exception']):
-            cut_into_subsequences(**params)
+            split(**params)
         return
 
-    arr = cut_into_subsequences(**params)
+    arr = split(**params)
 
     assert np.array_equal(arr, expected['value'], equal_nan=True), (
         f"arr = {arr}, expected = {expected['value']}"