Repository: neuroinformatics-unit/movement

Issue:
Remove equidistant time-spacing assumption in computing approximate derivative
 Currently when computing approximate derivatives, we assume equidistant time-spacing with a fixed `dt`:
https://github.com/neuroinformatics-unit/movement/blob/e7ccfe50f65b38dc1a05fc78e1a324df808d6198/movement/analysis/kinematics.py#L105-L113

We should instead provide the actual `time` axis values, i.e. `data.coords["time"].values`.
Since we do not need to compute derivatives along multiple axes (what [`np.gradient()`](https://numpy.org/doc/stable/reference/generated/numpy.gradient.html#numpy.gradient) can do and [`xr.differentiate()`](https://docs.xarray.dev/en/stable/generated/xarray.DataArray.differentiate.html) can't), an even simpler solution would be to use [`xr.differentiate()`](https://docs.xarray.dev/en/stable/generated/xarray.DataArray.differentiate.html) (as @sfmig has pointed out in #265), replacing the above lines with:
```python
for _ in range(order): 
     result = result.differentiate("time")
```


Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/neuroinformatics-unit/movement/pull/270?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=neuroinformatics-unit) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 99.76%. Comparing base [(`e7ccfe5`)](https://app.codecov.io/gh/neuroinformatics-unit/movement/commit/e7ccfe50f65b38dc1a05fc78e1a324df808d6198?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=neuroinformatics-unit) to head [(`2baa678`)](https://app.codecov.io/gh/neuroinformatics-unit/movement/commit/2baa67865d76edf1eb8b9e90cd0fa49cdc48a51b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=neuroinformatics-unit).

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


```diff
@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
- Coverage   99.76%   99.76%   -0.01%     
==========================================
  Files          14       14              
  Lines         866      863       -3     
==========================================
- Hits          864      861       -3     
  Misses          2        2              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/neuroinformatics-unit/movement/pull/270?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=neuroinformatics-unit).   
: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=neuroinformatics-unit).

sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=neuroinformatics-unit_movement&pullRequest=270) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=neuroinformatics-unit_movement&pullRequest=270&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=neuroinformatics-unit_movement&pullRequest=270&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=neuroinformatics-unit_movement&pullRequest=270&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=neuroinformatics-unit_movement&pullRequest=270&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=neuroinformatics-unit_movement&pullRequest=270&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=neuroinformatics-unit_movement&pullRequest=270)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=neuroinformatics-unit_movement&pullRequest=270) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=neuroinformatics-unit_movement&pullRequest=270&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=neuroinformatics-unit_movement&pullRequest=270&issueStatuses=ACCEPTED)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=neuroinformatics-unit_movement&pullRequest=270&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=neuroinformatics-unit_movement&pullRequest=270&metric=new_coverage&view=list)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=neuroinformatics-unit_movement&pullRequest=270&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=neuroinformatics-unit_movement&pullRequest=270)



Patch:
diff --git a/movement/analysis/kinematics.py b/movement/analysis/kinematics.py
index ed826cc..15375a5 100644
--- a/movement/analysis/kinematics.py
+++ b/movement/analysis/kinematics.py
@@ -1,6 +1,5 @@
 """Compute kinematic variables like velocity and acceleration."""
 
-import numpy as np
 import xarray as xr
 
 from movement.utils.logging import log_error
@@ -9,15 +8,17 @@ from movement.utils.logging import log_error
 def compute_displacement(data: xr.DataArray) -> xr.DataArray:
     """Compute displacement between consecutive positions.
 
-    This is the difference between consecutive positions of each keypoint for
-    each individual across time. At each time point ``t``, it's defined as a
-    vector in cartesian ``(x,y)`` coordinates, pointing from the previous
+    Displacement is the difference between consecutive positions
+    of each keypoint for each individual across ``time``.
+    At each time point ``t``, it is defined as a vector in
+    cartesian ``(x,y)`` coordinates, pointing from the previous
     ``(t-1)`` to the current ``(t)`` position.
 
     Parameters
     ----------
     data : xarray.DataArray
-        The input data containing ``time`` as a dimension.
+        The input data containing position information, with
+        ``time`` as a dimension.
 
     Returns
     -------
@@ -35,50 +36,62 @@ def compute_velocity(data: xr.DataArray) -> xr.DataArray:
     """Compute the velocity in cartesian ``(x,y)`` coordinates.
 
     Velocity is the first derivative of position for each keypoint
-    and individual across time. It's computed using numerical differentiation
-    and assumes equidistant time spacing.
+    and individual across ``time``, computed with the second order
+    accurate central differences.
 
     Parameters
     ----------
     data : xarray.DataArray
-        The input data containing ``time`` as a dimension.
+        The input data containing position information, with
+        ``time`` as a dimension.
 
     Returns
     -------
     xarray.DataArray
         An xarray DataArray containing the computed velocity.
 
+    See Also
+    --------
+    :py:meth:`xarray.DataArray.differentiate` : The underlying method used.
+
     """
-    return _compute_approximate_derivative(data, order=1)
+    return _compute_approximate_time_derivative(data, order=1)
 
 
 def compute_acceleration(data: xr.DataArray) -> xr.DataArray:
     """Compute acceleration in cartesian ``(x,y)`` coordinates.
 
-    Acceleration represents the second derivative of position for each keypoint
-    and individual across time. It's computed using numerical differentiation
-    and assumes equidistant time spacing.
+    Acceleration is the second derivative of position for each keypoint
+    and individual across ``time``, computed with the second order
+    accurate central differences.
 
     Parameters
     ----------
     data : xarray.DataArray
-        The input data containing ``time`` as a dimension.
+        The input data containing position information, with
+        ``time`` as a dimension.
 
     Returns
     -------
     xarray.DataArray
         An xarray DataArray containing the computed acceleration.
 
+    See Also
+    --------
+    :py:meth:`xarray.DataArray.differentiate` : The underlying method used.
+
     """
-    return _compute_approximate_derivative(data, order=2)
+    return _compute_approximate_time_derivative(data, order=2)
 
 
-def _compute_approximate_derivative(
+def _compute_approximate_time_derivative(
     data: xr.DataArray, order: int
 ) -> xr.DataArray:
     """Compute the derivative using numerical differentiation.
 
-    This assumes equidistant time spacing.
+    This function uses :py:meth:`xarray.DataArray.differentiate`,
+    which differentiates the array with the second order
+    accurate central differences.
 
     Parameters
     ----------
@@ -102,15 +115,8 @@ def _compute_approximate_derivative(
         raise log_error(ValueError, "Order must be a positive integer.")
     _validate_time_dimension(data)
     result = data
-    dt = data["time"].values[1] - data["time"].values[0]
     for _ in range(order):
-        result = xr.apply_ufunc(
-            np.gradient,
-            result,
-            dt,
-            kwargs={"axis": 0},
-        )
-    result = result.reindex_like(data)
+        result = result.differentiate("time")
     return result
 
 
@@ -124,11 +130,11 @@ def _validate_time_dimension(data: xr.DataArray) -> None:
 
     Raises
     ------
-    AttributeError
+    ValueError
         If the input data does not contain a ``time`` dimension.
 
     """
     if "time" not in data.dims:
         raise log_error(
-            AttributeError, "Input data must contain 'time' as a dimension."
+            ValueError, "Input data must contain 'time' as a dimension."
         )


Test patch:
diff --git a/tests/test_unit/test_kinematics.py b/tests/test_unit/test_kinematics.py
index e5241bc..1f75a82 100644
--- a/tests/test_unit/test_kinematics.py
+++ b/tests/test_unit/test_kinematics.py
@@ -53,7 +53,7 @@ class TestKinematics:
     kinematic_test_params = [
         ("valid_poses_dataset", does_not_raise()),
         ("valid_poses_dataset_with_nan", does_not_raise()),
-        ("missing_dim_poses_dataset", pytest.raises(AttributeError)),
+        ("missing_dim_poses_dataset", pytest.raises(ValueError)),
     ]
 
     @pytest.mark.parametrize("ds, expected_exception", kinematic_test_params)
@@ -109,4 +109,4 @@ class TestKinematics:
             ValueError if isinstance(order, int) else TypeError
         )
         with pytest.raises(expected_exception):
-            kinematics._compute_approximate_derivative(data, order=order)
+            kinematics._compute_approximate_time_derivative(data, order=order)