Skip to content

Fix resample_sig for signals starting with NaN - #604

Merged
tompollard merged 1 commit into
MIT-LCP:mainfrom
TheNetherWatcher:resample-leading-nan
Oct 31, 2026
Merged

tompollard merged 1 commit into
MIT-LCP:mainfrom
TheNetherWatcher:resample-leading-nan

Conversation

@TheNetherWatcher

Copy link
Copy Markdown
Contributor

resample_sig returns only NaN values if the signal starts with NaN:

import numpy as np
from wfdb.processing import resample_sig

resample_sig(np.array([np.nan, 1.0, 2.0, 3.0, 4.0, 5.0]), fs=2, fs_target=4)[0]
# array([nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan, nan])

NaN values are filled with pd.Series.interpolate() before resampling (added for #249), but by default it only fills forward. Trailing NaN values take the last valid value, while leading ones stay NaN, and the FFT resample then spreads them over the whole output. This also affects resample_singlechan and resample_multichan, for example on a record whose first samples are invalid.

The fix passes limit_direction="both", so leading NaN values take the first valid value. Inner and trailing NaN values are filled as before.

Tests: a signal with leading NaN values gives the same result as the signal with them filled (this fails on main), and a test for inner and trailing NaN values checks that they are still handled as before. pytest -m "not network" passes (98 tests), and the docs build passes.

pandas interpolate() only fills forward by default, so NaN values at the
start of a signal were left in place and the FFT resample spread them
over the whole output. Fill in both directions.

@tompollard tompollard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good! I checked that the leading-NaN test fails on main, that an all-NaN signal behaves as before, and that record 100 with its first 50 samples set to NaN now resamples to finite values.

One note for later, not for this PR: since the filled values are resampled like real data, the output has no NaN where the input was invalid. It might be worth marking those regions as NaN in the output at some point, but that's a separate change.

@tompollard
tompollard merged commit b0b5b5f into MIT-LCP:main Oct 31, 2026
18 checks passed
tompollard added a commit that referenced this pull request Oct 31, 2026
Follow-up to @tompollard's note on #604.

`resample_sig` fills NaN values before resampling, because the FFT would
otherwise spread them over the whole signal. But the filled values were
then returned as if they were real data, so the output had no trace of
where the input was missing.

Now output samples near a NaN input sample are set back to NaN. An
output sample at position `t` (in input samples) is NaN if any input
sample within half an output spacing of `t` is NaN (and within at least
one input sample, so neighbours count when upsampling). The filling and
resampling are unchanged, so samples away from the gaps keep the same
values.

For example, record 100 at 360 Hz with its first 50 samples set to NaN,
resampled to 250 Hz, gives NaN for the first 35 output samples
(positions below 50) and finite values after. Signals without NaN give
the same output as before.

This changes the output for signals with NaN, which is why it's a
separate PR. The two tests from #604 now expect NaN at the filled
positions.

Tests: new tests check which output samples are NaN when upsampling, and
when downsampling with a single NaN and with a gap between two output
samples; they fail without the change. `pytest -m "not network"` passes
(109 tests), and the docs build passes.
@TheNetherWatcher
TheNetherWatcher deleted the resample-leading-nan branch November 1, 2026 06:13
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.

2 participants