Added Option to Perform Alignment Using Only Ref. Matched Peaks#549
Merged
Added Option to Perform Alignment Using Only Ref. Matched Peaks#549
Conversation
Added UseRefMatchedPeaksOnly property to ParameterBase, AlignmentParameterSettingModel, GcmsAlignmentParameterSettingModel, AlignmentParameterSettingViewModel, and GcmsAlignmentParameterSettingViewModel to control peak matching behavior. Updated constructors and methods to initialize and synchronize this property.
Updated DataAccessor.cs to implement IFeatureAccessor<IMSScanProperty>. Added IFeatureAccessor<TScan> interface in IFeatureAccessor.cs with methods GetMSScanProperties and AccumulateChromatogram. Updated using statements in IFeatureAccessor.cs to include necessary namespaces.
Removed unused using directives from IPeakJoiner.cs to clean up code. Simplified IPeakJoiner interface by removing nested interface declaration and making Join method directly part of the interface. Fixed syntax error in PeakAligner.cs by removing an extra closing brace.
Refactored `LcmsPeakJoiner` and related classes to use `ChromatogramPeakFeature` instead of `IMSScanProperty`, improving type safety and maintainability. Updated `LcmsDataAccessor` to implement `IFeatureAccessor<ChromatogramPeakFeature>`. Enhanced `LcmsPeakJoiner` methods for better parameter handling and readability. Reorganized and updated `LcmsPeakJoinerTests` to reflect these changes, including the introduction of `StubAccessor` and `FakeEvaluator` classes. Added a new test method `Join_OnlyRefMatched` to verify functionality with `UseRefMatchedPeaksOnly` parameter. Updated project file to use the latest C# language version.
Updated GcmsAlignmentProcessFactory to include new parameters in CreateRTJoiner and CreateRIJoiner methods. Modified GcmsPeakJoiner to accept additional parameters and updated methods for null checks and list initialization. Enhanced GcmsRTPeakJoiner and GcmsRIPeakJoiner with new fields and constructors. Refactored GcmsDataAccessor to implement new interfaces and use switch expressions. Improved methods for merging spectrum features, progress reporting, and handling chromatogram data.
A new checkbox labeled "Do alignment for ref.matched peaks only:" was added to `AlignmentParameterSettingView.xaml`. It is bound to the `UseRefMatchedPeaksOnly.Value` property for its `IsChecked` state and uses the `IsReadOnly` property with a `NegativeConverter` to control its `IsEnabled` state. A tooltip explains its purpose: "Alignment is performed for peaks annotated." The checkbox is styled with properties like `HorizontalAlignment="Left"`, `VerticalContentAlignment="Center"`, and `Height="24"`.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds the UseRefMatchedPeaksOnly feature to control whether only reference‐matched peaks are used in the alignment process. The changes include refactoring peak joiners and data accessors to implement the new IFeatureAccessor interface using modern C# syntax, updated UI bindings and parameters, and enhanced test coverage for the new behavior.
- Refactored IPeakJoiner implementations (LCMS and GCMS) to use an AlignmentBaseParameter that controls filtering.
- Updated data accessors (e.g., LcmsDataAccessor, GcmsDataAccessor) to implement IFeatureAccessor with modern collection initializer expressions.
- Added new UI property bindings and tests to validate the UseRefMatchedPeaksOnly option.
Reviewed Changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MSDIAL5/MsdialLcMsApiTests/Algorithm/Alignment/LcmsPeakJoinerTests.cs | Refactored tests to use ChromatogramPeakFeature and updated the joiner instantiation; added new test for UseRefMatchedPeaksOnly. |
| src/MSDIAL5/MsdialLcMsApi/Algorithm/LcmsDataAccessor.cs | Updated to implement IFeatureAccessor with modern C# syntax. |
| src/MSDIAL5/MsdialLcMsApi/Algorithm/Alignment/LcmsPeakJoiner.cs | Refactored to use AlignmentBaseParameter for improved configuration and filtering logic. |
| src/MSDIAL5/MsdialGuiApp/ViewModel/Setting/*.cs | Added UI property and binding for UseRefMatchedPeaksOnly. |
| src/MSDIAL5/MsdialGcMsApi/Algorithm/Alignment/*.cs | Updated GCMS joiners and data accessors to support the new filtering and syntax improvements. |
| src/MSDIAL5/MsdialCore/Parameter/ParameterBase.cs | Added the UseRefMatchedPeaksOnly property in alignment parameters. |
| src/MSDIAL5/MsdialCore/Algorithm/IFeatureAccessor.cs | Introduced the IFeatureAccessor interface to standardize scan property retrieval. |
Files not reviewed (2)
- src/MSDIAL5/MsdialGuiApp/View/Setting/AlignmentParameterSettingView.xaml: Language not supported
- tests/MSDIAL5/MsdialLcMsApiTests/MsdialLcMsApiTests.csproj: Language not supported
Comments suppressed due to low confidence (2)
tests/MSDIAL5/MsdialLcMsApiTests/Algorithm/Alignment/LcmsPeakJoinerTests.cs:149
- Consider adding additional test cases that cover scenarios where no peaks are reference-matched when UseRefMatchedPeaksOnly is enabled, to ensure robust handling of an empty feature set.
public void Join_OnlyRefMatched() {
src/MSDIAL5/MsdialCore/Parameter/ParameterBase.cs:1160
- [nitpick] The property 'UseRefMatchedPeaksOnly' is clearly named; please ensure that its usage is fully consistent across the codebase to avoid any ambiguity.
public bool UseRefMatchedPeaksOnly { get; set; } = false;
Bujee415
approved these changes
Apr 28, 2025
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.
PR Classification
Refactoring, new feature, and test enhancements to improve modularity, maintainability, and alignment functionality.
PR Summary
This PR introduces the
UseRefMatchedPeaksOnlyfeature, refactors alignment logic to use modern C# practices, and enhances test coverage with updated data structures.IPeakJoiner,GcmsPeakJoiner, andLcmsPeakJoiner: Refactored to useIFeatureAccessorfor modularity and added support forUseRefMatchedPeaksOnly.GcmsDataAccessorandLcmsDataAccessor: ImplementedIFeatureAccessorand updated methods for better abstraction and readability.UseRefMatchedPeaksOnlyin alignment settings.LcmsPeakJoinerTeststo useChromatogramPeakFeature, introducedStubAccessorandFakeEvaluator, and added a new test forUseRefMatchedPeaksOnly.