Add ion match tolerance unit transition setting (Issue 830) #2156
Conversation
…k/20210927_issue830HighResCheckbox
Also, fix "AssertEx.NoDiff" so that it prepends provided helpMsg to failure message.
|
Thank you, Nick! Should I also add Skyline_22_14.xsd at this point? Or it is done at the release time? |
|
Brendan will create the file "Skyline_22_14.xsd" when he releases the next Skyline-Daily. |
| /// <summary>returns true iff a is in (b-tolerance, b+tolerance)</summary> | ||
| public static bool IsWithinTolerance(double a, double b, MzTolerance tolerance) | ||
| public bool IsWithinTolerance(double a, double b) |
There was a problem hiding this comment.
I have a question. Why the inequality is non-inclusive here while it is inclusive for the method match tolerance?
There was a problem hiding this comment.
Good question.
@chambm, is there a reason that this method is using an exclusive range?
I see that this C# method is not used anywhere but there is a C++ method in "MZTolerance.cpp" which has the same implementation.
Skyline always (as far as I know) interprets tolerances using an inclusive range so that if someone sets the tolerance to zero there will be one number which would still fall in the tolerance range.
…://github.com/ProteoWizard/pwiz into Skyline/work/20210927_issue830HighResCheckbox
- fix audit log format for MzTolerance to depend on RESX values Units_mz and Units_ppm - add handler for combo change event on TransitionSettingsUI.comboToleranceUnits change Method match tolerance by a factor of 1000
…in TransitionSettingsUI
…b.com:ProteoWizard/pwiz into Skyline/work/20210927_issue830HighResCheckbox
…TransitionLibraries from needing to test its values individually
…b.com:ProteoWizard/pwiz into Skyline/work/20210927_issue830HighResCheckbox
…controls consistent with the TransitionSettingsUI.Library tab
| SetZoom(false); | ||
| ClickChromatogram(33.11, 15.055, PaneKey.PRODUCTS); | ||
| WaitForGraphs(); | ||
| Assert.AreEqual(Skyline.Program.SkylineOffscreen? 70 : 20, SkylineWindow.GraphFullScan.IonLabels.Count()); |
There was a problem hiding this comment.
Why are these numbers different on-screen vs. off-screen? This at least needs a comment explaining the issue.
There was a problem hiding this comment.
The labels are not drawn in off-screen mode because it happens during Paint event. In the off-screen mode this tests total number of peaks matching the ion type filter. I'll add a comment in the code.
There was a problem hiding this comment.
Another approach can be to implement a method that gets the ion labels that would be added if the graph were painted. But, looking for a good example, I just ran into another case where we simply look at the CurveList for TextObj items. And I can see some benefit to testing the text that actually ends up on the graph. So, I can live with this.
| Settings.Default.ShowSpecialIons = false; | ||
| } | ||
|
|
||
| private void TestIonMatchToleranceUnitSetting() |
There was a problem hiding this comment.
To test this more thoroughly will require using the TransitionSettingsUI form and also a case of using PPM in the Import Peptide Search wizard to exercise the TransitionSettingsControl.
There was a problem hiding this comment.
I replaced the changing the settings with invocation of the Settings dialogue and added tolerance unit verification to the ImportPeptideSearchTest.
| @@ -334,7 +338,7 @@ private void TestIonMatchToleranceUnitSetting() | |||
| }); | |||
| WaitForGraphs(); | |||
| var graphLabels = SkylineWindow.GraphFullScan.IonLabels; | |||
| Assert.AreEqual(Skyline.Program.SkylineOffscreen ? 49 : 1, graphLabels.Count()); | |||
| Assert.AreEqual(Skyline.Program.SkylineOffscreen ? 48 : 1, graphLabels.Count()); | |||
There was a problem hiding this comment.
This number has changed because calling the settings dialogue resets the special ion list and there was one reporter ion in this spectrum.
| SetZoom(false); | ||
| ClickChromatogram(33.11, 15.055, PaneKey.PRODUCTS); | ||
| WaitForGraphs(); | ||
| Assert.AreEqual(Skyline.Program.SkylineOffscreen? 70 : 20, SkylineWindow.GraphFullScan.IonLabels.Count()); |
There was a problem hiding this comment.
Another approach can be to implement a method that gets the ion labels that would be added if the graph were painted. But, looking for a good example, I just ran into another case where we simply look at the CurveList for TextObj items. And I can see some benefit to testing the text that actually ends up on the graph. So, I can live with this.
| @@ -621,6 +622,8 @@ private void TestIrts() | |||
| Assert.IsTrue(importPeptideSearchDlg.CurrentPage == ImportPeptideSearchDlg.Pages.match_modifications_page); | |||
| Assert.IsTrue(importPeptideSearchDlg.ClickNextButton()); | |||
| Assert.IsTrue(importPeptideSearchDlg.CurrentPage == ImportPeptideSearchDlg.Pages.transition_settings_page); | |||
| importPeptideSearchDlg.TransitionSettingsControl.IonMatchToleranceUnits = MzTolerance.Units.ppm; | |||
There was a problem hiding this comment.
Test the prior value gets multiplied by 1000.
| SkylineWindow.ModifyDocument("Set test settings", | ||
| doc => doc.ChangeSettings(newSettings)); | ||
| transitionSettingsUI.IonMatchToleranceUnits = MzTolerance.Units.ppm; | ||
| transitionSettingsUI.IonMatchTolerance = 10.0; |
There was a problem hiding this comment.
Test the prior value gets multiplied by 1000 before changing it.
No description provided.