Conversation
Updated `AlignmentGnpsExporter` to use `IFileClassMetaAccessor` for improved abstraction. Modified `AlignmentGnpsExportModel` constructor to accept additional metadata accessors. Split `IsSelected` into `IsSelectedGnps` and `IsSelectedGnps2` for better control over export format selection. Refactored `CountExportFiles` and `Export` methods for clarity. Updated XAML UI to include new checkboxes for GNPS export formats and adjusted `AlignmentGnpsExportViewModel` accordingly.
Modified the ExportAsync method in the AlignmentReferenceMatchedProductIonExportModel class to include a check for the IsSelected property. If IsSelected is false, the method returns early, ensuring that the export process only proceeds when the object is in a selected state.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the GNPS export functionality by adding support for two distinct GNPS export formats: GNPS2 (new default) and GNPS (MSDIAL4 format). The implementation introduces interface abstraction for metadata accessors and updates the UI to allow users to select between export formats.
Key Changes:
- Added dual GNPS export format support with separate selection checkboxes (GNPS2 and GNPS/MSDIAL4 format)
- Introduced
IFileClassMetaAccessorinterface for better abstraction of metadata access - Updated UI to display both export options in a two-column grid layout
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| AlignmentGnpsExporter.cs | Changed to use IFileClassMetaAccessor interface instead of concrete GnpsFileClassMetaAccessor class |
| AlignmentGnpsExportModel.cs | Refactored to support two export formats with separate metadata accessors and selection properties; split export logic into format-specific methods |
| AlignmentGnpsExportViewModel.cs | Updated to expose IsSelectedGnps and IsSelectedGnps2 properties for binding to UI checkboxes |
| AlignmentResultExportWin.xaml | Modified GNPS section UI to show two checkboxes in a grid layout for selecting export formats |
| LcmsMethodModel.cs, LcimmsMethodModel.cs, ImmsMethodModel.cs, GcmsMethodModel.cs, DimsMethodModel.cs | Updated constructor calls to pass additional metadata accessor parameters |
| AlignmentReferenceMatchedProductIonExportModel.cs | Added early return guard for IsSelected check in async export method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| private void ExportForGnps(AlignmentFileBeanModel alignmentFile, string exportDirectory, Action<string> notification) { | ||
| var dt = DateTime.Now; | ||
| var outNameTemplate = $"{{0}}_{((IFileBean)alignmentFile).FileID}_{dt:yyyy_MM_dd_HH_mm_ss}"; |
There was a problem hiding this comment.
The variable outNameTemplate is declared but never used in the ExportForGnps method. This unused variable should be removed to improve code clarity.
| var outNameTemplate = $"{{0}}_{((IFileBean)alignmentFile).FileID}_{dt:yyyy_MM_dd_HH_mm_ss}"; |
|
|
||
| private void ExportForGnps2(AlignmentFileBeanModel alignmentFile, string exportDirectory, Action<string> notification) { | ||
| var dt = DateTime.Now; | ||
| var outNameTemplate = $"{{0}}_{((IFileBean)alignmentFile).FileID}_{dt:yyyy_MM_dd_HH_mm_ss}"; |
There was a problem hiding this comment.
The variable outNameTemplate is declared but never used in the ExportForGnps2 method. This unused variable should be removed to improve code clarity.
| var outNameTemplate = $"{{0}}_{((IFileBean)alignmentFile).FileID}_{dt:yyyy_MM_dd_HH_mm_ss}"; |
| var count = 0; | ||
| if (IsSelectedGnps) { | ||
| count += Types.Count(type => type.ShouldExport); | ||
| } | ||
| if (IsSelectedGnps2) { | ||
| count += Types.Count(type => type.ShouldExport); |
There was a problem hiding this comment.
The Types.Count(type => type.ShouldExport) enumeration is potentially executed twice. Consider caching this value in a local variable before the conditionals to avoid redundant enumeration if both formats are selected.
| var count = 0; | |
| if (IsSelectedGnps) { | |
| count += Types.Count(type => type.ShouldExport); | |
| } | |
| if (IsSelectedGnps2) { | |
| count += Types.Count(type => type.ShouldExport); | |
| var shouldExportCount = Types.Count(type => type.ShouldExport); | |
| var count = 0; | |
| if (IsSelectedGnps) { | |
| count += shouldExportCount; | |
| } | |
| if (IsSelectedGnps2) { | |
| count += shouldExportCount; |
| private void ExportForGnps(AlignmentFileBeanModel alignmentFile, string exportDirectory, Action<string> notification) { | ||
| var dt = DateTime.Now; |
There was a problem hiding this comment.
The notification parameter is declared but never used in the ExportForGnps method. Consider removing it if not needed, or implement notification calls if user feedback is intended.
| } | ||
| } | ||
|
|
||
| private void ExportForGnps2(AlignmentFileBeanModel alignmentFile, string exportDirectory, Action<string> notification) { |
There was a problem hiding this comment.
The notification parameter is declared but never used in the ExportForGnps2 method. Consider removing it if not needed, or implement notification calls if user feedback is intended.
| private void ExportForGnps2(AlignmentFileBeanModel alignmentFile, string exportDirectory, Action<string> notification) { | |
| private void ExportForGnps2(AlignmentFileBeanModel alignmentFile, string exportDirectory) { |
PR Classification
Enhancement of GNPS export functionality and UI updates.
PR Summary
This pull request refines the GNPS export process by introducing interfaces for better abstraction and adding new selection properties for different export formats. It also updates the UI to accommodate these changes and improves the organization of the export logic.
AlignmentGnpsExporter.cs: Updated to useIFileClassMetaAccessorand added methods for handling different GNPS export formats.AlignmentGnpsExportModel.cs: Modified constructor to include new metadata accessors and updated selection properties.AlignmentGnpsExportViewModel.cs: Reflects changes in the model with new properties for GNPS selection states.AlignmentResultExportWin.xaml: UI updated to include checkboxes for new GNPS export formats.