Check that QRS indices are increasing in compute_hr (#603)

Fixes #602.

`compute_hr` now raises a `ValueError` if `qrs_inds` is not strictly
increasing, naming the first pair that isn't, for example:

```
ValueError: qrs_inds must be strictly increasing, but qrs_inds[1] = 8 is followed by qrs_inds[2] = 8
```

Before, repeated beats gave a `ZeroDivisionError` (lists) or `inf` with
a `RuntimeWarning` (arrays), and beats out of order gave a negative
heart rate. The docstring now says the indices must be strictly
increasing and documents the error.


Tests: repeated, decreasing and repeated-last-beat inputs (lists and
arrays) raise the error. `pytest -m "not network"` passes (103 tests),
and the docs build passes.

Author: tompollard

docs/changes.rst

@@ -32,6 +32,9 @@ Unreleased
 **Use the last RR interval in compute_hr**
   ``compute_hr`` now uses the interval between the last two beats for the heart rate after the last beat, instead of repeating the previous rate. With exactly two beats it now returns their rate after the second beat instead of only NaN.
 
+**Check that QRS indices are increasing in compute_hr**
+  ``compute_hr`` now raises a ``ValueError`` if ``qrs_inds`` is not strictly increasing. Repeated beats used to give a ``ZeroDivisionError`` (or ``inf`` for NumPy arrays), and beats out of order gave a negative heart rate.
+
 **Use Ruff for formatting and linting**
   The ``dev`` extra now includes ``ruff`` instead of ``black`` and ``pylint``, and the code is formatted with a line length of 88 (the default).
 

src/wfdb/processing/hr.py

@@ -18,7 +18,7 @@ def compute_hr(sig_len, qrs_inds, fs):
     sig_len : int
         The length of the corresponding signal.
     qrs_inds : ndarray
-        The QRS index locations.
+        The QRS index locations, in strictly increasing order.
     fs : int, float
         The corresponding signal's sampling frequency.
 
@@ -29,7 +29,22 @@ def compute_hr(sig_len, qrs_inds, fs):
         corresponding signal. Contains numpy.nan where heart rate could
         not be computed.
 
+    Raises
+    ------
+    ValueError
+        If `qrs_inds` is not strictly increasing.
+
     """
+    # Compare neighbours directly, since np.diff wraps for unsigned integers.
+    q = np.asarray(qrs_inds)
+    not_increasing = np.flatnonzero(q[1:] <= q[:-1])
+    if not_increasing.size:
+        i = not_increasing[0]
+        raise ValueError(
+            "qrs_inds must be strictly increasing, but "
+            f"qrs_inds[{i}] = {q[i]} is followed by qrs_inds[{i + 1}] = {q[i + 1]}"
+        )
+
     heart_rate = np.full(sig_len, np.nan, dtype="float32")
 
     if len(qrs_inds) < 2:

tests/test_processing.py

@@ -1,6 +1,7 @@
 from unittest import mock
 
 import numpy as np
+import pytest
 
 import wfdb
 from wfdb import processing
@@ -204,6 +205,23 @@ def test_last_beat_past_end(self):
         expected[7:] = 60 * 60 / 4
         np.testing.assert_allclose(hr, expected)
 
+    @pytest.mark.parametrize("qrs_inds", [[2, 8, 8, 15], [2, 8, 5, 15], [2, 8, 8]])
+    def test_rejects_unordered_indices(self, qrs_inds):
+        """Repeated or decreasing beats raise a ValueError."""
+        with pytest.raises(ValueError, match="strictly increasing"):
+            processing.compute_hr(sig_len=20, qrs_inds=qrs_inds, fs=1)
+
+    def test_rejects_unordered_array(self):
+        """The check also applies to NumPy arrays."""
+        with pytest.raises(ValueError, match=r"qrs_inds\[1\] = 8"):
+            processing.compute_hr(sig_len=20, qrs_inds=np.array([2, 8, 8]), fs=1)
+
+    def test_rejects_unordered_unsigned(self):
+        """Decreasing unsigned indices are caught, although np.diff wraps."""
+        qrs_inds = np.array([2, 8, 5], dtype=np.uint32)
+        with pytest.raises(ValueError, match=r"qrs_inds\[1\] = 8"):
+            processing.compute_hr(sig_len=20, qrs_inds=qrs_inds, fs=1)
+
 
 class TestQrs:
     """Testing QRS detectors"""