Internal msfinder batch and single processing update#571
Internal msfinder batch and single processing update#571YukiMatsuzawa merged 21 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the internal MS-FINDER batch and single-spot workflows by adding user feedback windows, introducing a substructure-aware spectrum view, and modernizing/refactoring core model logic and utilities.
- Show progress messages during long-running operations and replace cursor overrides with popup windows
- Add
SmilesToStructureImageConverterandMsSpectrumViewWithSubstructureImagefor inline structure previews - Refactor
MsfinderObservedMetabolite,InternalMsFinder,StructureFinder, andFormulaGeneratorfor consistent naming and C# style
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| InternalMsfinderBatchSettingViewModel.cs | Wrapped batch/single runs with message windows; removed AddTo(Disposables) calls |
| SmilesToStructureImageConverter.cs | New converter to render SMILES as images in the spectrum view |
| InternalMsFinderView.xaml | Swapped in the new MsSpectrumViewWithSubstructureImage; corrected property bindings |
| MsfinderObservedMetabolite.cs | Reworked formula/structure lists to use VMs; added filtering callbacks and updated spectrum handling |
| InternalMsFinder.cs | Updated axis labels to use fragmentation score; wired in new chart control |
| StructureFinder.cs | Renamed methods to PascalCase and cleaned up I/O utilities |
| FormulaGenerator.cs | Renamed private fields/methods to readonly/PascalCase and adopted collection expressions |
Comments suppressed due to low confidence (5)
src/MSDIAL5/MsdialGuiApp/View/Search/InternalMsFinderView.xaml:394
- Property name casing is inconsistent (
SubstructureInChIkeysvs.SubstructureInChIKeys); it may fail at runtime if the VM property is Pascal-cased.
TextBox Text="{Binding Path=Model.SelectedObservedMetabolite.SelectedStructure.SubstructureInChIkeys, Mode=OneWay}"
src/MSDIAL5/MsdialGuiApp/ViewModel/Setting/InternalMsfinderBatchSettingViewModel.cs:56
- The new
ReactiveCommandsubscriptions are no longer disposed via.AddTo(Disposables); consider re-adding disposal to avoid potential memory leaks.
Run = new ReactiveCommand().WithSubscribe(() => {
src/MSDIAL5/MsdialGuiApp/Model/Search/InternalMsFinder.cs:50
- [nitpick] UI axis label should use consistent capitalization (e.g. "Fragmentation Score") to match other chart titles.
"fragmentation score"
src/MSDIAL5/MsdialGuiApp/Model/Search/MsfinderObservedMetabolite.cs:410
- Looping over all formula results in
ShowProductIonSpectrummay produce unintended aggregated spectra; it seems you intended to use the selected formula (SelectedFormula) instead.
foreach (var formula in FormulaList) {
src/MSDIAL5/MsdialGuiApp/Model/Search/MsfinderObservedMetabolite.cs:437
- The inner loop duplicates each neutral-loss peak
Counttimes; remove the extraforso each peak is added only once.
for (var i = 0; i < neutralLossList.Count; i++) {
| } | ||
| return eSpectrum; | ||
| } | ||
| //private ObservableCollection<double[]> GetExperimentalSpectrum(RawData rawData, AnalysisParamOfMsfinder param) |
There was a problem hiding this comment.
[nitpick] Large blocks of commented-out legacy code reduce readability; consider removing or moving to a separate branch if you need to preserve it.
No description provided.