Skip to content

Document what the XQRS backsearch skip depends on #597

Closed

Description

@tompollard

#596 made XQRS._backsearch skip peaks that an earlier backsearch already rejected. This is safe because of two properties of the current code:

  • _update_noise doesn't recalculate qrs_thr, so a backsearch result only changes after _update_qrs.
  • _update_qrs sets last_qrs_peak_num = self.peak_num (the current peak) rather than the peak_num argument.

Both look like things someone might change later, for example to match Pan-Tompkins more closely. If someone does, the skip could break without any test catching it.

The comment above the skip in _backsearch should name these dependencies, for example:

# Skip peaks a previous backsearch already rejected. A backsearch result
# depends only on qrs_thr and last_qrs_ind, which change only in
# _update_qrs, and _update_qrs moves last_qrs_peak_num to the current
# peak. If either of these stops being true, remove this skip.

Activity

  1. TheNetherWatcher commented on Oct 31, 2026

    @TheNetherWatcher
    Contributor

    Thanks for writing this up, and for checking #596 so carefully. I've opened #598: it uses your comment and adds a test that checks both properties during detection. It fails if either one is changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions