skyline: Stopped inferring a Waters lockmass function from waters_connect mzML #4498
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -318,11 +318,15 @@ PWIZ_API_DECL SpectrumList_FilterPredicate_MSLevelSet::SpectrumList_FilterPredic | |
|
|
||
| PWIZ_API_DECL boost::logic::tribool SpectrumList_FilterPredicate_MSLevelSet::accept(const msdata::Spectrum& spectrum) const | ||
| { | ||
| // A spectrum may carry more than one child of "spectrum type" - a Waters lockspray scan is both an | ||
| // MS1 spectrum and a calibration spectrum - and no rule fixes the order a writer emits them in, so | ||
| // asking only for the first child could take a real MS1 for a non-MS spectrum and drop it. Match | ||
| // SpectrumList_FilterPredicate_AnalyzerType::accept below, which already asks this way. | ||
| if (!spectrum.hasCVParamChild(MS_spectrum_type)) | ||
|
Member
There was a problem hiding this comment. Shouldn't this be reverted since it was only need for multiple spectrum types?
Member
Author
There was a problem hiding this comment. I think this is a different change than the one you have in mind - the multiple-spectrum-types one was reverted, back in Phase 6 when the writing half of this PR was withdrawn. Nothing in pwiz emits two children of What is here now fixes a calibration-spectrum-only spectrum, which is what CVID spectrumType = rawIndexEntry.frameType == FrameType_Calibration ? MS_calibration_spectrum : ...
result->set(MS_ms_level, msLevel);
result->set(spectrumType);
Worth noting that restoring the reverted hunk would not have fixed this - it asked It does become moot, but only once psi-ms #539 lands and UIMF is updated to write Mutation-verified: restoring the type-first ordering fails
Member
Author
There was a problem hiding this comment. oof, AI slop. I shouldn't let Claude handle the responses directly
Member
There was a problem hiding this comment. This is going to break with the new CV, when calibration spectrum is no longer a spectrum type. (Also the comment is wrong here about multiple spectrum types.) Fixing UIMF in this PR is the right path I think, which doesn't fix existing UIMF files, but we could fix those broken UIMF files in the mzML reader. Which I'm inclined say we should do so since they are OUR broken files. In untested code:
Member
There was a problem hiding this comment. Although I guess it's not just mzML. Someone could have converted to any of the open mzML formats like mz5, mzMLb, perhaps even mzXML. Tricky. Maybe a centralized workaround location (I don't think we've had a case like this before) and then call it in mzML, mz5, and mzMLb right before they return a spectrum.
Member
There was a problem hiding this comment. Which is possibly more effort than the UIMF format warrants, as I haven't heard of anybody using it recently (a similar fate to mz5 and mzMLb, sigh). I'm sure multi-parquet-in-a-zip mzPeak will change things though. /s We could put some opt-in telemetry into pwiz that tracks what formats people use, input and output. |
||
| return boost::logic::indeterminate; | ||
| if (!spectrum.hasCVParamChild(MS_mass_spectrum)) | ||
| return msLevelSet_.contains(0); // non-MS spectra are considered ms level 0 | ||
| CVParam param = spectrum.cvParam(MS_ms_level); | ||
| if (param.cvid == CVID_Unknown) return boost::logic::indeterminate; | ||
| int msLevel = param.valueAs<int>(); | ||
| bool result = msLevelSet_.contains(msLevel); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.