Skip to content

Fix map_peaks aborting on the first unmapped compound#26

Merged
gchure merged 2 commits into
mainfrom
fix/map-peaks-missing-compound
Jun 22, 2026
Merged

Fix map_peaks aborting on the first unmapped compound#26
gchure merged 2 commits into
mainfrom
fix/map-peaks-missing-compound

Conversation

@gchure

@gchure gchure commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

This PR addresses #23. Chromatogram.map_peaks() stopped mapping as soon as it hit a requested compound with no peak within loc_tolerance. Because the loop used break, every compound listed after the missing one was silently skipped — and if the first compound was the missing one, mapper stayed empty and the method raised ValueError("No peaks could be properly mapped!") even when all the other compounds were present in the chromatogram.

This was reported in #23 with a reproducing example.

Root cause

In map_peaks (hplc/quant.py), the "no peak found" branch exited the whole loop:

if np.sum(peak_id) == 0:
    unmapped[k] = v["retention_time"]
    break          # <-- skips every remaining compound

Fix

Change break to continue so each present compound is mapped and each absent compound still records an entry in unmapped and emits its warning.

Tests

  • Adds a regression test where a missing compound precedes valid ones and asserts the valid compounds are still mapped. (Fails on main, passes here.)
  • Full suite green.

Risk

Minimal — single-token logic change endorsed by the reporter and maintainer in #23. Existing test_map_peaks continues to pass (its missing-compound case is last in the dict, so it was unaffected by the original bug).

map_peaks() used `break` when a requested compound had no peak within
loc_tolerance, which silently skipped every compound listed after the
missing one and raised "No peaks could be properly mapped!" when the
first compound was the missing one. Change `break` to `continue` so all
present compounds are mapped and each absent compound still warns.

Adds a regression test where a missing compound precedes valid ones.

Fixes #23

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.12%. Comparing base (391f9ee) to head (93a69be).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #26   +/-   ##
=======================================
  Coverage   97.12%   97.12%           
=======================================
  Files           3        3           
  Lines         556      556           
=======================================
  Hits          540      540           
  Misses         16       16           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Use a set comparison for mapped compounds (robust to peak count and
lexicographic ordering, e.g. compound_10 vs compound_2) and scope the
warning assertion to the specific "No peak found for absent" message
rather than matching any warning.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gchure
gchure merged commit 522c61e into main Jun 22, 2026
5 checks passed
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.

1 participant