From a07bb68b0277860efa0ea6b983d008ff3b1a4766 Mon Sep 17 00:00:00 2001 From: Bharat Kaurav <117659673+TheNetherWatcher@users.noreply.github.com> Date: Sat, 31 Oct 2026 22:46:25 +0530 Subject: [PATCH 1/2] Check that QRS indices are increasing in compute_hr Repeated beats gave a ZeroDivisionError (or inf for NumPy arrays), and beats out of order gave a negative heart rate. Raise a ValueError naming the first pair that is not increasing. Fixes #602. --- docs/changes.rst | 3 +++ src/wfdb/processing/hr.py | 16 +++++++++++++++- tests/test_processing.py | 12 ++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/docs/changes.rst b/docs/changes.rst index 07a89c58..51a281b7 100644 --- a/docs/changes.rst +++ b/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). diff --git a/src/wfdb/processing/hr.py b/src/wfdb/processing/hr.py index 4aba8510..68b195bc 100644 --- a/src/wfdb/processing/hr.py +++ b/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,21 @@ 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. + """ + not_increasing = np.flatnonzero(np.diff(qrs_inds) <= 0) + if not_increasing.size: + i = not_increasing[0] + raise ValueError( + "qrs_inds must be strictly increasing, but " + f"qrs_inds[{i}] = {qrs_inds[i]} is followed by " + f"qrs_inds[{i + 1}] = {qrs_inds[i + 1]}" + ) + heart_rate = np.full(sig_len, np.nan, dtype="float32") if len(qrs_inds) < 2: diff --git a/tests/test_processing.py b/tests/test_processing.py index 78409d2e..3030117c 100644 --- a/tests/test_processing.py +++ b/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,17 @@ 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) + class TestQrs: """Testing QRS detectors""" From 4c58440945dd9ebc052a210861f2c1c39d3d97a8 Mon Sep 17 00:00:00 2001 From: Bharat Kaurav <117659673+TheNetherWatcher@users.noreply.github.com> Date: Sat, 31 Oct 2026 23:08:10 +0530 Subject: [PATCH 2/2] Compare QRS indices directly instead of with np.diff np.diff wraps around for unsigned integers, so decreasing unsigned indices weren't caught. --- src/wfdb/processing/hr.py | 7 ++++--- tests/test_processing.py | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/wfdb/processing/hr.py b/src/wfdb/processing/hr.py index 68b195bc..004401b0 100644 --- a/src/wfdb/processing/hr.py +++ b/src/wfdb/processing/hr.py @@ -35,13 +35,14 @@ def compute_hr(sig_len, qrs_inds, fs): If `qrs_inds` is not strictly increasing. """ - not_increasing = np.flatnonzero(np.diff(qrs_inds) <= 0) + # 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}] = {qrs_inds[i]} is followed by " - f"qrs_inds[{i + 1}] = {qrs_inds[i + 1]}" + 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") diff --git a/tests/test_processing.py b/tests/test_processing.py index 3030117c..325decec 100644 --- a/tests/test_processing.py +++ b/tests/test_processing.py @@ -216,6 +216,12 @@ def test_rejects_unordered_array(self): 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"""