feat: implement iter_chromatograms (TIC + BPC) for mzML export - #26
Merged
Nathan-D-R merged 3 commits intoJul 26, 2026
Merged
Conversation
Wire SpectrumSource::iter_chromatograms on TdfSource / OwnedTdfSource,
which previously fell back to the trait's default empty iterator, so
OpenTimsTDF's mzML output now carries a <chromatogramList>.
Two whole-run traces are emitted, one point per frame in acquisition
order, built purely from columns Reader::frames already reads (no extra
SQL, no peak decode):
- TIC ("total ion current chromatogram", MS:1000235) from
Frames.SummedIntensities (Frame::summed_intensities), the
accumulation-normalized per-frame intensity sum.
- BPC ("basepeak chromatogram", MS:1000628) from Frames.MaxIntensity,
a new additive column added to both Frames SELECTs and mapped onto a
new Frame::max_intensity field (also exposed on the Python Frame).
A trace whose source column is absent on every frame is omitted rather
than emitted empty, matching the writer's contract of only emitting a
chromatogramList when the source yields something.
SRM/PRM transition chromatograms are intentionally left out: they need
per-target cross-frame aggregation of the decoded peak stream, not just
column wiring. Distinct from Sigilweaver#13 (PRM spectra), which this does not
touch.
Closes Sigilweaver#25.
Claude-Session: https://claude.ai/code/session_01TLSifJpmaN7RvW3xi2uWgo
# Conflicts: # CHANGELOG.md
3 tasks
Nathan-D-R
pushed a commit
that referenced
this pull request
Jul 28, 2026
Wire the remaining piece of iter_chromatograms scoped out of #26: one SRM-shaped ChromatogramRecord ("selected reaction monitoring chromatogram", MS:1001473) per scheduled PrmTarget, built on top of the TIC/BPC traces #26 already wired up. For each PrmTarget, walk every msms_type == 10 (prm-PASEF) frame, decode its peak payload via the same per-frame decode path iter_spectra already uses (Reader::decode_peaks), sum the decoded intensity restricted to that target's PrmFrameMsMsInfo scan-number range, and append one point per frame. A target with a genuine zero-intensity frame keeps that point (real data, unlike a frame missing a Frames column); a target absent from every PrmFrameMsMsInfo row is omitted rather than emitted with an empty trace, matching the TIC/BPC no-empty-chromatogram contract. precursor_mz is the target's scheduled PrmTargets.MonoisotopicMz. product_mz is left unset: prm-PASEF records a full fragment-ion scan per precursor rather than a discrete triple-quadrupole product-ion transition, and the TDF schema has no product-ion m/z field to report - populating one would mean inventing a value with no schema backing, which the clean-room policy in CONTRIBUTING.md rules out. New pure helpers (sum_intensity_in_range, build_srm_record) are unit tested directly; the frame-walking orchestration (srm_chromatograms_for) follows the same shape as the existing spectra_for_frame and isn't unit tested in isolation for the same reason that one isn't - it's thin glue over already-tested pieces and a live Reader/bundle. The existing cache-gated prm_pasef_pxd028279 test in tests/roundtrip.rs already exercises the metadata this relies on (PrmFrameMsMsInfo/PrmTargets shape) and continues to pass. Distinct from #13 (prm-PASEF frames skipped in the mzML *spectrum* projection): that issue is about spectra, this is about the chromatogram trace. Addresses the remaining SRM/PRM scope of #25 (TIC/BPC portion done in #26). Claude-Session: https://claude.ai/code/session_018ijUinjq3SKNZXFNWqTTmV
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
2 participants
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.
Related to #25 (TIC/BPC portion; SRM/PRM chromatograms remain open in that issue, retitled and scoped accordingly - see the issue comment).
Summary
TdfSource/OwnedTdfSourcepreviously usedSpectrumSource::iter_chromatograms's default empty implementation, so OpenTimsTDF's mzML output had no<chromatogramList>(theopenmassspec-corewriter only emits it when the source yields something). This wires up the two chromatogram traces that can be built from dataReader::framesalready reads, following the scope/format of the referenced OpenWRaw#9 precedent (map existing decoded data intoopenmassspec_core::ChromatogramRecord, omit channels that can't be confidently mapped rather than guessing).Both traces are whole-run, one point per frame in acquisition order, in seconds. No extra SQL beyond a single added column, no peak decode.
TIC (decoded-but-unused data)
MS:1000235) fromFrame::summed_intensities(Frames.SummedIntensities), the accumulation-normalized per-frame intensity sum documented indocs/docs/format/01-tdf-sqlite-schema.mdand already cross-checked against the decoded peak sum bytests/roundtrip.rs. This column was previously decoded but read nowhere outside a test mock.BPC (small additive decode)
MS:1000628) fromFrames.MaxIntensity. This addsMaxIntensityto bothFramesSELECTs, a newFrame::max_intensityfield, and its mapping inframe_from_row. The column is already documented in the public schema's observed-columns block; a detailed table row for it was added here.A trace whose source column is absent on every frame is omitted rather than emitted empty, matching the writer's contract of only emitting a
<chromatogramList>when the source yields something.SRM/PRM: not included (suggested follow-up)
SRM/PRM transition chromatograms are intentionally left out. As the issue notes, producing an SRM-shaped
ChromatogramRecordperPrmTargetmeans walkingmsms_type == 10frames, decoding the peak payload restricted to each target's scan range, and summing intensity per frame - genuine per-target cross-frame aggregation, materially larger than wiring existing columns. It would fit well as its own follow-up issue/PR. This is distinct from #13 (PRM-PASEF spectra skipped in the spectrum projection): that is about spectra, this would be about the chromatogram trace; neither fixes the other.Also
max_intensityon the PythonFrame(kept symmetric withsummed_intensities), and documented it indocs/docs/reference/python-api.md.Testing
cargo fmt --all -- --check: clean.cargo clippy --all-targets -- -D warnings: clean.cargo test --all: all pass, including 4 new unit tests inmzml.rscovering TIC+BPC construction, seconds-preserving retention time, skip-frames-missing-a-column (arrays stay parallel), omit-empty-trace, and all-absent -> no records. The real-.d-bundle roundtrip/conformance tests are cache-gated and skip when the PRIDE cache is absent; correctness of the addedMaxIntensitySELECT is otherwise argued from the public schema.Clean-room
Per CONTRIBUTING.md: correctness is argued from the schema doc, the existing roundtrip/self-consistency invariants, and the PSI-MS accessions the
openmassspec-corewriter expects. No vendor software was used.Contributed by @Nabejo.