test(tools): pin find_peaks' four behaviours, and fix the docstring that described two of them wrongly - #816
Merged
Merged
Conversation
Mutation-testing #815 before merge showed the eight newly-collected spot-HRV tests catch a grossly broken peak detector but not two of find_peaks' three behaviours: deleting the min_dist spacing filter left all eight passing, and so did deleting the min_prom prominence gate. Only inverting the local-maximum comparison failed anything. #815 recorded that as a known gap; this closes it. test_find_peaks_counts_beats cannot cover either one by construction — its synthetic beats are already well separated and well above the noise floor, so both filters are no-ops on that input. These use minimal hand-built arrays instead, and each asserts BOTH directions so it cannot pass vacuously: min_dist [0,5,0,9,0], maxima two apart -> min_dist=3 keeps only the taller [3], min_dist=2 keeps both [1,3] min_prom [0,2,0,9,0] -> min_prom=5 rejects the 2.0 maximum, min_prom=1 keeps it Mutants now killed 3/3 where it was 1/3. Suite 167 -> 169, OK (1 skipped). Note for anyone repeating this: clear __pycache__ between mutations. A stale .pyc kept reporting failures from a mutant after the source had been restored, which reads exactly like a real regression in code that is actually clean.
…them wrongly Reviewing the previous commit turned up a docstring that contradicts the code it documents, on two counts: docstring: "Local maxima >= neighbours and >= min_prom" code: v[i] > v[i-1] and v[i] >= v[i+1] and v[i] > min_prom min_prom is a STRICT >, so a maximum sitting exactly on the threshold is rejected, not kept. And the neighbour test is ASYMMETRIC, not ">= neighbours": on a plateau [0,5,5,0] it returns [1], the left edge only. The code is right on both. The asymmetry is the standard way to take one index from a plateau — symmetric >= would emit a duplicate per plateau sample and inflate the beat count, symmetric > would drop plateaus entirely. So the docstring is corrected rather than the behaviour, and both boundaries are now pinned by tests: a peak exactly at min_prom is rejected, and a plateau of any length yields one index. Mutation matrix, all with __pycache__ cleared between runs: drop min_dist spacing killed drop min_prom gate killed min_prom > becomes >= killed (new) neighbours symmetric >= killed (new) neighbours symmetric > killed (new) 5/5, up from 1/3 before this branch. Suite 169 -> 170, OK (1 skipped). Worth noting the docstring had been wrong for as long as it existed and no test could have caught it, because nothing exercised either boundary. That is the same shape as the rest of today's tooling work: the gap was not a failing test, it was the absence of one.
ryanbr
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the coverage gap #815 recorded rather than fixed.
Mutation-testing #815 before merging showed its eight newly-collected spot-HRV tests catch a grossly broken peak detector but miss two of
find_peaks' three behaviours:find_peaksmin_distspacing filtermin_promprominence gatetest_find_peaks_counts_beatscan't cover either by construction — its synthetic beats are already well separated and well above the noise floor, so both filters are no-ops on that input. Passing it says nothing about whether they exist.These use minimal hand-built arrays, and each asserts both directions so it can't pass vacuously:
min_dist—[0, 5, 0, 9, 0], maxima two samples apart.min_dist=3keeps only the taller ([3]);min_dist=2keeps both ([1, 3]).min_prom—[0, 2, 0, 9, 0].min_prom=5.0rejects the 2.0 maximum;min_prom=1.0keeps it.Mutants killed 3/3, up from 1/3. Suite 167 → 169, OK (1 skipped).
One practical note for anyone repeating the exercise: clear
__pycache__between mutations. A stale.pyckept reporting failures from a mutant after the source had been restored — indistinguishable from a real regression in code that was actually clean.Tests only. No app code, no tooling behaviour, no schema, no strings.