Skip to content

fix: HeartRateExtractor.extract() PyO3 binding misnames Rust phases param as weights#1290

Open
mvanhorn wants to merge 1 commit into
ruvnet:mainfrom
mvanhorn:fix/1225-ruview-heartrate-phases-binding
Open

fix: HeartRateExtractor.extract() PyO3 binding misnames Rust phases param as weights#1290
mvanhorn wants to merge 1 commit into
ruvnet:mainfrom
mvanhorn:fix/1225-ruview-heartrate-phases-binding

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jul 9, 2026

Copy link
Copy Markdown

Summary

HeartRateExtractor.extract() now takes a phases argument that matches the Rust core, so the documented call path returns real estimates instead of always returning None.

Why this matters

Issue #1225 reports that hr.extract(residuals=..., weights=[]) always returns None. The PyO3 binding named the second parameter weights, but the Rust core requires per-subcarrier unwrapped phases:

// v2/crates/wifi-densepose-vitals/src/heartrate.rs:100
pub fn extract(&mut self, residuals: &[f64], phases: &[f64]) -> Option<VitalEstimate> {
    let n = residuals.len().min(self.n_subcarriers).min(phases.len());

Because the sample count is clamped by phases.len(), passing the documented weights=[] sets n = 0 and the extractor returns None for every frame. HeartRateExtractor is not BreathingExtractor: it needs phase data, so an empty second argument is never valid.

Changes

  • Rename the binding parameter weights -> phases and pass it straight through to the core, so the Python API name reflects what the DSP actually consumes.
  • Update the docstring with a runnable example and a note that phases=[] is invalid for heart-rate extraction.
  • Update the two keyword call sites in the test and benchmark suites.

Testing

Added test_heart_rate_extract_with_synthetic_signal_and_phases (drives a synthetic 1.2 Hz / 72 BPM signal with matching phase data and asserts a finite BPM and a confidence in [0, 1]) and test_heart_rate_extract_rejects_old_weights_keyword (asserts the removed weights= keyword now raises TypeError). The parameter rename matches the core signature at heartrate.rs:100.

Fixes #1225

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HeartRateExtractor.extract() always returns None with weights=[] — PyO3 binding misnames Rust phases param as weights (2.0.0a1)

1 participant