Add SpectrumLabel and label selection to UI #626
Conversation
- Updated DisplayScan to include SpectrumLabel for detailed peak information. - Changed ViewTemplates to bind chart:Annotator to SpectrumLabel. - Added ComboBox for selecting label type (m/z or NL) in the UI. - Introduced LabelProperty in SplitSpectrumsViewModel for binding.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces spectrum label functionality and UI controls for selecting label types in the spectrum viewer. The changes enable users to switch between displaying m/z values and neutral loss (NL) values as labels on spectrum peaks.
- Added SpectrumLabel property to DisplayScan for providing peak label data
- Modified chart annotations to bind to SpectrumLabel instead of Spectrum and support dynamic label selection
- Added ComboBox UI control for selecting between m/z and NL label types
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| SplitSpectrumsViewModel.cs | Added LabelProperty for binding label type selection |
| ViewTemplates.xaml | Updated chart annotators to use SpectrumLabel and added ComboBox for label selection |
| DisplayScan.cs | Added SpectrumLabel property with anonymous objects containing Mass, Intensity, and NeutralLoss |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| public ChromXs ChromXs { get => Scan.ChromXs; set => Scan.ChromXs = value; } | ||
| public IonMode IonMode { get => Scan.IonMode; set => Scan.IonMode = value; } | ||
| public double PrecursorMz { get => Scan.PrecursorMz; set => Scan.PrecursorMz = value; } | ||
| public object[] SpectrumLabel => Scan.Spectrum.Select(p => new { p.Mass, p.Intensity, NeutralLoss = p.Mass - Scan.PrecursorMz }).ToArray(); |
There was a problem hiding this comment.
This property creates a new array every time it's accessed. Consider caching the result or making it a method to make the performance cost explicit, especially if this property is bound to UI controls that may access it frequently.
| public object[] SpectrumLabel => Scan.Spectrum.Select(p => new { p.Mass, p.Intensity, NeutralLoss = p.Mass - Scan.PrecursorMz }).ToArray(); | |
| public object[] GetSpectrumLabel() { | |
| return Scan.Spectrum.Select(p => new { p.Mass, p.Intensity, NeutralLoss = p.Mass - Scan.PrecursorMz }).ToArray(); | |
| } |
| public ChromXs ChromXs { get => Scan.ChromXs; set => Scan.ChromXs = value; } | ||
| public IonMode IonMode { get => Scan.IonMode; set => Scan.IonMode = value; } | ||
| public double PrecursorMz { get => Scan.PrecursorMz; set => Scan.PrecursorMz = value; } | ||
| public object[] SpectrumLabel => Scan.Spectrum.Select(p => new { p.Mass, p.Intensity, NeutralLoss = p.Mass - Scan.PrecursorMz }).ToArray(); |
There was a problem hiding this comment.
Using anonymous objects and object[] creates a weakly-typed API that's prone to runtime errors. Consider creating a dedicated class or struct (e.g., SpectrumLabelData) with Mass, Intensity, and NeutralLoss properties for better type safety.
Add SpectrumLabel and label selection to UI