Skip to content

Conversation

bspratt
Copy link
Member

@bspratt bspratt commented Aug 7, 2024

In https://skyline.ms/announcements/home/issues/exceptions/thread.view?entityId=14a60bd2-3604-103d-a666-22f53556a042&_anchor=65275 a call to ViewLibraryDlg.AddAllPeptides with an water loss adduct on a molecule with no O in it throws an exception. Instead, let's just skip it and put a warning in the immediate window. (Ideally it wouldn't be in the library in the first place, but we don't have any data on how the library was created)

Note that a water loss is tolerated for molecules described as mass only (we just deduct the mass of H2O), but when there's a chemical formula available the adduct really needs to make sense.

bspratt added 3 commits August 7, 2024 15:15
…xceptions/thread.view?entityId=14a60bd2-3604-103d-a666-22f53556a042&_anchor=65275 a call to ViewLibraryDlg.AddAllPeptides with an water loss adduct on a molecule with no O in it throws an exception. Instead, let's just skip it and put a warning in the immediate window. (Ideally it wouldn't be in the library in the first place, but we don't have any data on how the library was created)
@nickshulman
Copy link
Contributor

"PeptideTipProvider" is where we have been trying to put the code that informs the user that there is something wrong with the library entry.
The user will see that the entry in the Library Explorer does not have an icon next to it, and when they hover over it they will see a tooltip which could explain the problem.

@bspratt
Copy link
Member Author

bspratt commented Aug 10, 2024

"PeptideTipProvider" is where we have been trying to put the code that informs the user that there is something wrong with the library entry. The user will see that the entry in the Library Explorer does not have an icon next to it, and when they hover over it they will see a tooltip which could explain the problem.

Excellent, I'll add a check for adduct applicability there.

public class InvalidChemicalModificationException : IOException
which is thrown when a loss would remove more atoms than are found in a molecular formula e.g. water loss from C2S5H12 (no oxygen), or label would change more atoms than are in the formula e.g. 3C' on C2S5H12

We now catch this exception in LibraryExplorer and adjust display accordingly
@bspratt
Copy link
Member Author

bspratt commented Aug 14, 2024

image

@bspratt
Copy link
Member Author

bspratt commented Aug 14, 2024

Hmm, just noticed that the bad precursor gets neither the protein nor the molecule icon. Should it get the molecule anyway, or maybe some kind of alarm icon?

@nickshulman
Copy link
Contributor

Hmm, just noticed that the bad precursor gets neither the protein nor the molecule icon. Should it get the molecule anyway, or maybe some kind of alarm icon?

Anything that has an error is supposed to get no icon.
In the Library Explorer tutorial it says:

Some of the listed peptides show a peptide icon to the left of the sequence, while others do not. The
icon indicates that the current document modification settings match the modified state of the peptide
in the library.

@bspratt
Copy link
Member Author

bspratt commented Sep 24, 2024

@nickshulman can you give this the blessing? Thanks

@bspratt
Copy link
Member Author

bspratt commented Oct 8, 2024

Brendan had a couple of changes - show the spectra anyway, and make the explanation prominent:

image

@bspratt
Copy link
Member Author

bspratt commented Nov 13, 2024

Good idea:

image

@brendanx67
Copy link
Contributor

Good idea:

image

What triggers the text that is appearing in the Immediate window? Why is it appearing 3 times in a library with 1 entry? Will it appear now for any entry that causes a red hint in its tooltip, including proteomics?

…_mismatch' of https://github.com/ProteoWizard/pwiz into Skyline/work/20240807_graceful_handling_adduct_molecule_mismatch
@bspratt
Copy link
Member Author

bspratt commented Nov 13, 2024

What triggers the text that is appearing in the Immediate window? Why is it appearing 3 times in a library with 1 entry? Will it appear now for any entry that causes a red hint in its tooltip, including proteomics?

Currently it only happens when a InvalidChemicalModificationException is caught in pwiz.Skyline.Model.AbstractModificationMatcher.CreateDocNodeFromSetting, and that's small molecule only.

But yes, that gets called 3 times when you open Library Explorer. I should probably add some logic to the the Listener implementation to ignore repeating messages within, say, .5 seconds of each other.

Call stack 1:

	pwiz.CommonUtil.dll!pwiz.Common.SystemUtil.Messages.WriteAsyncUserMessage(string message, object[] args) Line 42	C#
	Skyline-daily.exe!pwiz.Skyline.Model.AbstractModificationMatcher.CreateDocNodeFromSettings(pwiz.Skyline.Model.Lib.LibKey key, pwiz.Skyline.Model.Peptide peptide, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff diff, out pwiz.Skyline.Model.TransitionGroupDocNode nodeGroupMatched) Line 508	C#
 	Skyline-daily.exe!pwiz.Skyline.Model.LibKeyModificationMatcher.GetModifiedNode(pwiz.Skyline.Model.Lib.LibKey key, pwiz.Skyline.Model.DocSettings.SrmSettings settings, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff diff) Line 344	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryPepMatching.AssociateMatchingPeptide(pwiz.Skyline.Model.Lib.ViewLibraryPepInfo pepInfo, pwiz.Skyline.Util.Adduct charge, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff settingsDiff) Line 349	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateListPeptide.AnonymousMethod__0(pwiz.Skyline.Util.ILongWaitBroker longWaitBroker) Line 597	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.LongOperationRunner.RunOnThisThread(System.Action<pwiz.Skyline.Util.ILongWaitBroker> performWork) Line 106	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.LongOperationRunner.Run(System.Action<pwiz.Skyline.Util.ILongWaitBroker> action) Line 49	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateListPeptide(int selectPeptideIndex) Line 584	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.FilterAndUpdate() Line 1072	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.comboFilterCategory_SelectedIndexChanged(object sender, System.EventArgs e) Line 1390	C#
 	[External Code]	
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.InitializeMatchCategoryComboBox() Line 245	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.InitializePeptides() Line 474	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateViewLibraryDlg() Line 378	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.ViewLibraryDlg_Load(object sender, System.EventArgs e) Line 285	C#
 	[External Code]	
 	pwiz.CommonUtil.dll!pwiz.Common.SystemUtil.CommonFormEx.OnLoad(System.EventArgs e) Line 48	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.FormEx.OnLoad(System.EventArgs e) Line 150	C#
 	[External Code]	
 	Skyline-daily.exe!pwiz.Skyline.SkylineWindow.OpenLibraryExplorer(string libraryName) Line 1925	C#
 	Skyline-daily.exe!pwiz.Skyline.SkylineWindow.ViewSpectralLibraries() Line 1894	C#
 	Skyline-daily.exe!pwiz.Skyline.Menus.ViewMenu.ViewSpectralLibraries() Line 187	C#
 	Skyline-daily.exe!pwiz.Skyline.Menus.ViewMenu.spectralLibrariesToolStripMenuItem_Click(object sender, System.EventArgs e) Line 182	C#

Call stack 2:

	pwiz.CommonUtil.dll!pwiz.Common.SystemUtil.Messages.WriteAsyncUserMessage(string message, object[] args) Line 42	C#
	Skyline-daily.exe!pwiz.Skyline.Model.AbstractModificationMatcher.CreateDocNodeFromSettings(pwiz.Skyline.Model.Lib.LibKey key, pwiz.Skyline.Model.Peptide peptide, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff diff, out pwiz.Skyline.Model.TransitionGroupDocNode nodeGroupMatched) Line 508	C#
 	Skyline-daily.exe!pwiz.Skyline.Model.LibKeyModificationMatcher.GetModifiedNode(pwiz.Skyline.Model.Lib.LibKey key, pwiz.Skyline.Model.DocSettings.SrmSettings settings, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff diff) Line 344	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryPepMatching.AssociateMatchingPeptide(pwiz.Skyline.Model.Lib.ViewLibraryPepInfo pepInfo, pwiz.Skyline.Util.Adduct charge, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff settingsDiff) Line 349	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateListPeptide.AnonymousMethod__0(pwiz.Skyline.Util.ILongWaitBroker longWaitBroker) Line 597	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.LongOperationRunner.RunOnThisThread(System.Action<pwiz.Skyline.Util.ILongWaitBroker> performWork) Line 106	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.LongOperationRunner.Run(System.Action<pwiz.Skyline.Util.ILongWaitBroker> action) Line 49	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateListPeptide(int selectPeptideIndex) Line 584	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateViewLibraryDlg() Line 382	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.ViewLibraryDlg_Load(object sender, System.EventArgs e) Line 285	C#
 	[External Code]	
 	pwiz.CommonUtil.dll!pwiz.Common.SystemUtil.CommonFormEx.OnLoad(System.EventArgs e) Line 48	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.FormEx.OnLoad(System.EventArgs e) Line 150	C#
 	[External Code]	
 	Skyline-daily.exe!pwiz.Skyline.SkylineWindow.OpenLibraryExplorer(string libraryName) Line 1925	C#
 	Skyline-daily.exe!pwiz.Skyline.SkylineWindow.ViewSpectralLibraries() Line 1894	C#
 	Skyline-daily.exe!pwiz.Skyline.Menus.ViewMenu.ViewSpectralLibraries() Line 187	C#

Call stack 3:

	pwiz.CommonUtil.dll!pwiz.Common.SystemUtil.Messages.WriteAsyncUserMessage(string message, object[] args) Line 42	C#
	Skyline-daily.exe!pwiz.Skyline.Model.AbstractModificationMatcher.CreateDocNodeFromSettings(pwiz.Skyline.Model.Lib.LibKey key, pwiz.Skyline.Model.Peptide peptide, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff diff, out pwiz.Skyline.Model.TransitionGroupDocNode nodeGroupMatched) Line 508	C#
 	Skyline-daily.exe!pwiz.Skyline.Model.LibKeyModificationMatcher.GetModifiedNode(pwiz.Skyline.Model.Lib.LibKey key, pwiz.Skyline.Model.DocSettings.SrmSettings settings, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff diff) Line 344	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryPepMatching.AssociateMatchingPeptide(pwiz.Skyline.Model.Lib.ViewLibraryPepInfo pepInfo, pwiz.Skyline.Util.Adduct charge, pwiz.Skyline.Model.DocSettings.SrmSettingsDiff settingsDiff) Line 349	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateListPeptide.AnonymousMethod__0(pwiz.Skyline.Util.ILongWaitBroker longWaitBroker) Line 597	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.LongOperationRunner.RunOnThisThread(System.Action<pwiz.Skyline.Util.ILongWaitBroker> performWork) Line 106	C#
 	Skyline-daily.exe!pwiz.Skyline.Util.LongOperationRunner.Run(System.Action<pwiz.Skyline.Util.ILongWaitBroker> action) Line 49	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.UpdateListPeptide(int selectPeptideIndex) Line 584	C#
 	Skyline-daily.exe!pwiz.Skyline.SettingsUI.ViewLibraryDlg.ViewLibraryDlg_Shown(object sender, System.EventArgs e) Line 292	C#

This isn't specific to use in Library Explorer, so if this condition is hit in a commandline context the user is made aware of it.

Copy link
Contributor

@brendanx67 brendanx67 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not merge this. Seems overly intrusive to me, and potentially very irritating to a user with a large library used in a document that targets only a fraction of what is in the library. The test case is a library with only 1 molecule and it reports the same error 3 times.

I am totally fine with just the implementation that shows the error message in the tooltip, but not yet with verbose output potentially where not output is requested or desired.

@bspratt
Copy link
Member Author

bspratt commented Nov 13, 2024

Agreed, too noisy as is. I'm working on TraceListener logic to avoid repeat messages. But I do think that in a large library these messages have value - you hear about problems without having to manually page through the entries.

@bspratt
Copy link
Member Author

bspratt commented Nov 13, 2024

That message is deduped now:
image

…ary entries are filtered out as being invalid instead of just doing it silently
@bspratt
Copy link
Member Author

bspratt commented Nov 13, 2024

Once you get started with these ansynchronous messages, you see that it's really useful...

image

…k/20240807_graceful_handling_adduct_molecule_mismatch
@bspratt bspratt merged commit 11b9e99 into master Oct 22, 2025
11 checks passed
@bspratt bspratt deleted the Skyline/work/20240807_graceful_handling_adduct_molecule_mismatch branch October 22, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants