More graceful handling of libraries containing nonsensical entries (e.g. water loss on molecule containing no oxygen atoms) #3107
Conversation
…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)
…duct_molecule_mismatch
|
"PeptideTipProvider" is where we have been trying to put the code that informs the user that there is something wrong with the library entry. |
Excellent, I'll add a check for adduct applicability there. |
…duct_molecule_mismatch
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
|
|
…duct_molecule_mismatch
|
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.
|
…_mismatch' of https://github.com/ProteoWizard/pwiz into Skyline/work/20240807_graceful_handling_adduct_molecule_mismatch
…duct_molecule_mismatch
…duct_molecule_mismatch
…_mismatch' of https://github.com/ProteoWizard/pwiz into Skyline/work/20240807_graceful_handling_adduct_molecule_mismatch
…duct_molecule_mismatch
* Fix a bug in m/z calculation where the mass of the adduct was not being added to molecules described only as a mass with no chemical formula. (#3151)
…k/20240807_graceful_handling_adduct_molecule_mismatch
…duct_molecule_mismatch
…duct_molecule_mismatch
|
@nickshulman can you give this the blessing? Thanks |
…duct_molecule_mismatch
…duct_molecule_mismatch
…duct_molecule_mismatch
|
Brendan had a couple of changes - show the spectra anyway, and make the explanation prominent:
|
…duct_molecule_mismatch
Show the spectrum even if m/z can't be calculated
…duct_molecule_mismatch
…_mismatch' of https://github.com/ProteoWizard/pwiz into Skyline/work/20240807_graceful_handling_adduct_molecule_mismatch
…duct_molecule_mismatch
…duct_molecule_mismatch
…duct_molecule_mismatch
…duct_molecule_mismatch
|
@nickshulman @brendanx67 if that last screenshot looks good I'd really like to get this merged. |
| } | ||
| catch (InvalidChemicalModificationException e) | ||
| { | ||
| Messages.WriteAsyncUserMessage(e.Message); // Adduct makes no sense for target formula |
There was a problem hiding this comment.
When this message shows up in the Immediate Window it does not have enough information for the user to be able to figure out where the error is coming from.
In the document that I sent you, the message says:
Adduct "[M-2H2O+K]" calls for removing more O atoms than are found in the molecule C8H8O
The message should contain the name of the spectral library.
Also, it should probably also say the name of the molecule in addition to the chemical formula because the chemical formula is not what you see in the Library Explorer.
Here's the document:
adductmismatch.sky.zip
…not make sense for library molecule
|
Good idea:
|
…duct_molecule_mismatch
…duct_molecule_mismatch
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
Currently it only happens when a 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: Call stack 2: Call stack 3: 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. |
There was a problem hiding this comment.
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.
|
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. |
|
That message is deduped now: |
…ary entries are filtered out as being invalid instead of just doing it silently
|
Once you get started with these ansynchronous messages, you see that it's really useful...
|
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.