-
Notifications
You must be signed in to change notification settings - Fork 47
Add Precursor and PrecursorSet classes for m/z tolerance–based precursor management #2553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces new infrastructure for managing precursor features with m/z tolerance-based deduplication in the MetaMorpheus engine. The implementation provides a foundation for upcoming workflows requiring unique precursor handling such as external deconvolution pairing and MRM precursor management.
Key changes include:
- Introduction of an immutable
Precursor
record class to standardize precursor data representation - Implementation of a
PrecursorSet
class that maintains collections of unique precursors based on m/z tolerance - Comprehensive test coverage for both classes with edge cases and boundary conditions
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
MetaMorpheus/EngineLayer/PrecursorSet.cs | Main implementation containing both Precursor record and PrecursorSet class with tolerance-based uniqueness logic |
MetaMorpheus/EngineLayer/Precursor.cs | Empty file that appears to be a placeholder |
MetaMorpheus/Test/PrecursorSetTests.cs | Comprehensive unit tests covering all functionality including tolerance checking, bucket management, and edge cases |
MetaMorpheus/EngineLayer/Ms2ScanWithSpecificMass.cs | Minor formatting change adding blank line |
Comments suppressed due to low confidence (1)
@@ -0,0 +1 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file contains only a BOM character and appears to be a placeholder. If the Precursor record is defined in PrecursorSet.cs instead, this empty file should be removed to avoid confusion.
|
Copilot uses AI. Check for mistakes.
namespace EngineLayer | ||
{ | ||
public record Precursor(double MonoisotopicPeakMz, int Charge, double Mass, double Intensity, int EnvelopePeakCount, double? FractionalIntensity = null); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The Precursor record is defined in the same file as PrecursorSet. Consider moving it to its own file (Precursor.cs) for better organization and maintainability, especially since there's already a Precursor.cs file in the project.
Copilot uses AI. Check for mistakes.
return false; | ||
} | ||
|
||
integerKey = (int)Math.Round(precursor.MonoisotopicPeakMz * 100.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 100.0 for m/z scaling appears multiple times in the code. Consider extracting this as a named constant (e.g., private const double MZ_SCALE_FACTOR = 100.0) to improve maintainability and make the scaling factor more explicit.
integerKey = (int)Math.Round(precursor.MonoisotopicPeakMz * 100.0); | |
integerKey = (int)Math.Round(precursor.MonoisotopicPeakMz * MzScaleFactor); |
Copilot uses AI. Check for mistakes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2553 +/- ##
=======================================
Coverage 96.63% 96.64%
=======================================
Files 158 159 +1
Lines 23854 23900 +46
Branches 3322 3330 +8
=======================================
+ Hits 23052 23097 +45
- Misses 797 798 +1
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Summary
This PR introduces two new classes to the
EngineLayer
namespace that provide a standardized way to represent precursor features and manage collections of unique precursors. These additions will be used in upcoming workflows that require deduplication of precursor lists (e.g., external decon pairing, MRM selected precursor handling).Key Changes
Precursor
classImmutable representation of a precursor feature, storing:
MonoisotopicPeakMz
Mass
Charge
Intensity
EnvelopePeakCount
FractionalIntensity
Constructor enforces initialization of all required fields.
PrecursorSet
classIEnumerable<Precursor>
for easy iteration.Add
andContainsEquivalent
methods for controlled set membership.Motivation
Currently, there is no unified way in MetaMorpheus to store and deduplicate precursors with m/z-based uniqueness rules. These classes encapsulate that logic in a reusable, testable form, enabling future features like:
Next Steps
PrecursorSet
into theGetMs2Scans
workflow for per-scan precursor list construction.Precursor
andPrecursorSet
in the new MRM and external decon features currently in development.