feat: implement iter_chromatograms (TIC + BPC) for mzML export#26
Open
Nabejo wants to merge 1 commit into
Open
feat: implement iter_chromatograms (TIC + BPC) for mzML export#26Nabejo wants to merge 1 commit into
Nabejo wants to merge 1 commit into
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
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
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.
Fixes #25.
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.