diff --git a/pwiz_tools/Shared/zedgraph/ZedGraph/CurveList.cs b/pwiz_tools/Shared/zedgraph/ZedGraph/CurveList.cs index 41ac58eae3..4eeb07e1e5 100644 --- a/pwiz_tools/Shared/zedgraph/ZedGraph/CurveList.cs +++ b/pwiz_tools/Shared/zedgraph/ZedGraph/CurveList.cs @@ -477,7 +477,7 @@ public void GetRange( bool bIgnoreInitial, bool isBoundedRanges, GraphPane pane pane._barSettings.Base == BarBase.X2 ) { // Only force z=0 for BarItems, not HiLowBarItems - if ( ! (curve is HiLowBarItem) ) + if (!yScale.IsLog && !(curve is HiLowBarItem)) { if ( tYMinVal > 0 ) tYMinVal = 0; @@ -496,7 +496,7 @@ public void GetRange( bool bIgnoreInitial, bool isBoundedRanges, GraphPane pane else { // Only force z=0 for BarItems, not HiLowBarItems - if ( !( curve is HiLowBarItem ) ) + if (!xScale.IsLog && !(curve is HiLowBarItem )) { if ( tXMinVal > 0 ) tXMinVal = 0; diff --git a/pwiz_tools/Skyline/Controls/Graphs/AreaPeptideGraphPane.cs b/pwiz_tools/Skyline/Controls/Graphs/AreaPeptideGraphPane.cs index cf1de6c9da..706b6ebc05 100644 --- a/pwiz_tools/Skyline/Controls/Graphs/AreaPeptideGraphPane.cs +++ b/pwiz_tools/Skyline/Controls/Graphs/AreaPeptideGraphPane.cs @@ -46,7 +46,15 @@ protected override void UpdateAxes() var maxY = GraphHelper.GetMaxY(CurveList, this); GraphHelper.ReformatYAxis(this, maxY); - FixedYMin = YAxis.Scale.Min = Settings.Default.AreaLogScale ? 1 : 0; + if (Settings.Default.AreaLogScale) + { + YAxis.Scale.MinAuto = true; + FixedYMin = null; + } + else + { + FixedYMin = 0; + } } internal class AreaGraphData : GraphData diff --git a/pwiz_tools/Skyline/Controls/Graphs/AreaReplicateGraphPane.cs b/pwiz_tools/Skyline/Controls/Graphs/AreaReplicateGraphPane.cs index 1e8f44456a..82098b0c8d 100644 --- a/pwiz_tools/Skyline/Controls/Graphs/AreaReplicateGraphPane.cs +++ b/pwiz_tools/Skyline/Controls/Graphs/AreaReplicateGraphPane.cs @@ -194,6 +194,11 @@ private static BarType BarType return BarType.PercentStack; } + if (Settings.Default.AreaLogScale && AreaGraphController.AreaNormalizeOption.AllowLogScale) + { + return BarType.Cluster; + } + return BarType.Stack; } } @@ -653,11 +658,6 @@ public override void UpdateGraph(bool selectionChanged) ParentGroupNode = parentGroupNode; SumAreas = sumAreas; - // Draw a box around the currently selected replicate - if (ShowSelection && maxArea > -double.MaxValue) - { - AddSelection(normalizeOption, selectedReplicateIndex, sumArea, maxArea); - } // Reset the scale when the parent node changes bool resetAxes = (_parentNode == null || !ReferenceEquals(_parentNode.Id, parentNode.Id)); _parentNode = parentNode; @@ -692,6 +692,11 @@ public override void UpdateGraph(bool selectionChanged) AddDotProductLine(graphData); UpdateAxes(resetAxes, aggregateOp, dataScalingOption, normalizeOption); + // Draw a box around the currently selected replicate + if (ShowSelection && maxArea > -double.MaxValue) + { + AddSelection(normalizeOption, selectedReplicateIndex, sumArea, maxArea); + } } private void AddDotProductLine(AreaGraphData graphData) @@ -832,12 +837,7 @@ private void AddSelection(NormalizeOption areaView, int selectedReplicateIndex, } else { - GraphObjList.Add(new BoxObj(selectedReplicateIndex + .5, yValue, 0.99, - -yValue, Color.Black, Color.Empty) - // Just passing in yValue here doesn't work when log scale is enabled, -yValue works with and without log scale enabled - { - IsClippedToChartRect = true, - }); + DrawSelectionBox(selectedReplicateIndex, yValue, 0); } } @@ -923,11 +923,9 @@ private void UpdateAxes(bool resetAxes, GraphValues.AggregateOp aggregateOp, Dat YAxis.Scale.MinAuto = false; FixedYMin = YAxis.Scale.Min = 0; } - else if (Settings.Default.AreaLogScale) + else if (Settings.Default.AreaLogScale && BarSettings.Type == BarType.Cluster) { - // If currently not log scale, reset the y-axis max - if (YAxis.Type != AxisType.Log) - YAxis.Scale.MaxAuto = true; + YAxis.Scale.MaxAuto = true; if (Settings.Default.PeakAreaMaxArea != 0) { YAxis.Scale.MaxAuto = false; @@ -936,8 +934,8 @@ private void UpdateAxes(bool resetAxes, GraphValues.AggregateOp aggregateOp, Dat YAxis.Type = AxisType.Log; YAxis.Title.Text = GraphValues.AnnotateLogAxisTitle(GetYAxisTitle(aggregateOp, normalizeOption)); - YAxis.Scale.MinAuto = false; - FixedYMin = YAxis.Scale.Min = 1; + YAxis.Scale.MinAuto = true; + FixedYMin = null; } else { @@ -964,16 +962,20 @@ private void UpdateAxes(bool resetAxes, GraphValues.AggregateOp aggregateOp, Dat YAxis.Scale.MaxAuto = true; } Legend.IsVisible = !IsMultiSelect && Settings.Default.ShowPeakAreaLegend; + RemoveInvalidPointValues(); AxisChange(); // Reformat Y-Axis for labels and whiskers var maxY = GraphHelper.GetMaxY(CurveList,this); - if (DotProductLabelsVisible || DotProductLineVisible) + if ((DotProductLabelsVisible || DotProductLineVisible) && !YAxis.Scale.IsLog) { var reservedSpace = _labelHeight * 2; var unitsPerPixel = maxY / (Chart.Rect.Height - reservedSpace); - var extraSpace = reservedSpace*unitsPerPixel; - maxY += extraSpace; + if (unitsPerPixel > 0) + { + var extraSpace = reservedSpace * unitsPerPixel; + maxY += extraSpace; + } } GraphHelper.ReformatYAxis(this, maxY > 0 ? maxY : 0.1); // Avoid same min and max, since it blanks the entire graph pane diff --git a/pwiz_tools/Skyline/Controls/Graphs/GraphHelper.cs b/pwiz_tools/Skyline/Controls/Graphs/GraphHelper.cs index 9c0a949e7a..18f73b56d2 100644 --- a/pwiz_tools/Skyline/Controls/Graphs/GraphHelper.cs +++ b/pwiz_tools/Skyline/Controls/Graphs/GraphHelper.cs @@ -579,18 +579,34 @@ public static void FormatFontSize(GraphPane g, float fontSize) g.Legend.FontSpec.Size = fontSize; } + /// + /// Ensure that the maximum value displayed on the Y-axis is greater than . + /// This is used when the maximum value is different from what ZedGraph thinks it should be, such as because + /// of the extra space for in or the space required + /// for the dot product lines and labels. + /// public static void ReformatYAxis(GraphPane g, double myMaxY) { - var _max = MyMod(myMaxY, g.YAxis.Scale.MajorStep) == 0.0 ? myMaxY : - myMaxY + g.YAxis.Scale.MajorStep - MyMod(myMaxY, g.YAxis.Scale.MajorStep); - g.YAxis.Scale.Max = _max; - } - protected static double MyMod(double x, double y) - { - if (y == 0) - return 0; - var temp = x / y; - return y * (temp - Math.Floor(temp)); + var yAxisScale = g.YAxis.Scale; + double newMax; + if (yAxisScale.IsLog) + { + newMax = Math.Pow(10.0, Math.Ceiling(Math.Log10(myMaxY))); + } + else + { + var majorStep = yAxisScale.MajorStep; + if (majorStep <= 0) + { + return; + } + newMax = Math.Ceiling(myMaxY / majorStep) * majorStep; + } + + if (newMax != yAxisScale.Max && newMax > yAxisScale.Min) + { + yAxisScale.Max = newMax; + } } // Find maximum value for bar graph including whiskers diff --git a/pwiz_tools/Skyline/Controls/Graphs/SummaryBarGraphPaneBase.cs b/pwiz_tools/Skyline/Controls/Graphs/SummaryBarGraphPaneBase.cs index 8bb227e202..8008b215db 100644 --- a/pwiz_tools/Skyline/Controls/Graphs/SummaryBarGraphPaneBase.cs +++ b/pwiz_tools/Skyline/Controls/Graphs/SummaryBarGraphPaneBase.cs @@ -410,6 +410,102 @@ protected bool IsRepeatRemovalAllowed set { _axisLabelScaler.IsRepeatRemovalAllowed = value; } } + protected void RemoveInvalidPointValues() + { + if (!YAxis.Scale.IsLog) + { + return; + } + + foreach (var curve in CurveList) + { + if (curve.IsY2Axis) + { + continue; + } + + curve.Points = NonPositiveToMissing(curve.Points); + } + } + + /// + /// Replace with PointPairBase.Missing all points which do not have a positive y coordinate + /// and cannot be correctly displayed on a log scale. + /// + protected static IPointList NonPositiveToMissing(IPointList pointList) + { + int nPts = pointList.Count; + if (!Enumerable.Range(0, nPts).Any(i => pointList[i].Y <= 0)) + { + return pointList; + } + + var newPoints = new List(pointList.Count); + for (int i = 0; i < nPts; i++) + { + var pt = pointList[i]; + if (pt.Y <= 0) + { + newPoints.Add(new PointPair(pt.X, PointPairBase.Missing)); + } + else + { + newPoints.Add(pt); + } + } + + return new PointPairList(newPoints); + } + + protected void DrawSelectionBox(int xValue, double yMax, double yMin) + { + if (YAxis.Scale.IsLog) + { + // If it's a log scale, the bottom should be controlled by the lowest value displayed + var minPositiveYValue = GetMinPositiveYValue(); + if (!minPositiveYValue.HasValue) + { + return; + } + + // The Y-axis is usually zoomed to Math.Pow(10.0, Math.Floor(Math.Log10(minPositiveYValue.Value))) + // Make the selection box extend 3 orders of magnitude below the lowest value so that it goes all the way + // even if the user zooms out a bit. + yMin = minPositiveYValue.Value / 1000; + } + GraphObjList.Add(new BoxObj(xValue + .5, yMax, 0.99, + yMin - yMax, Color.Black, Color.Empty) + { + IsClippedToChartRect = true, + }); + } + + protected double? GetMinPositiveYValue() + { + double? minValue = null; + foreach (var curve in CurveList) + { + if (curve.IsY2Axis) + { + continue; + } + + for (int i = 0; i < curve.NPts; i++) + { + var pt = curve.Points[i]; + if (pt.Y > 0) + { + if (!minValue.HasValue || minValue.Value > pt.Y) + { + minValue = pt.Y; + } + } + } + } + + return minValue; + } + #region Test Support Methods public string[] GetOriginalXAxisLabels() { diff --git a/pwiz_tools/Skyline/Controls/Graphs/SummaryPeptideGraphPane.cs b/pwiz_tools/Skyline/Controls/Graphs/SummaryPeptideGraphPane.cs index 07a41bbc1c..612469da4c 100644 --- a/pwiz_tools/Skyline/Controls/Graphs/SummaryPeptideGraphPane.cs +++ b/pwiz_tools/Skyline/Controls/Graphs/SummaryPeptideGraphPane.cs @@ -141,19 +141,11 @@ public override void UpdateGraph(bool selectionChanged) CurveList.Add(curveItem); } + UpdateAxes(); if (ShowSelection && SelectedIndex != -1) { - double yValue = _graphData.SelectedMaxY; - double yMin = _graphData.SelectedMinY; - double height = yValue - yMin; - GraphObjList.Add(new BoxObj(SelectedIndex + .5, yValue, 0.99, - height, Color.Black, Color.Empty) - { - IsClippedToChartRect = true, - }); + DrawSelectionBox(SelectedIndex, _graphData.SelectedMaxY, _graphData.SelectedMinY); } - - UpdateAxes(); } protected abstract GraphData CreateGraphData(SrmDocument document, PeptideGroupDocNode selectedProtein, @@ -170,8 +162,8 @@ protected void UpdateAxes(bool allowLogScale) { YAxis.Title.Text = TextUtil.SpaceSeparate(GraphsResources.SummaryPeptideGraphPane_UpdateAxes_Log, YAxis.Title.Text); YAxis.Type = AxisType.Log; - YAxis.Scale.MinAuto = false; - FixedYMin = YAxis.Scale.Min = 1; + YAxis.Scale.MinAuto = true; + FixedYMin = null; YAxis.Scale.Max = _graphData.MaxY * 10; } else @@ -227,7 +219,7 @@ protected void UpdateAxes(bool allowLogScale) XAxis.Scale.TextLabels = _graphData.Labels; ScaleAxisLabels(); - + RemoveInvalidPointValues(); AxisChange(); } diff --git a/pwiz_tools/Skyline/Model/Results/NormalizeOption.cs b/pwiz_tools/Skyline/Model/Results/NormalizeOption.cs index 42915a2d27..c364b47e3e 100644 --- a/pwiz_tools/Skyline/Model/Results/NormalizeOption.cs +++ b/pwiz_tools/Skyline/Model/Results/NormalizeOption.cs @@ -68,6 +68,19 @@ public bool IsRatioToLabel get { return NormalizationMethod is NormalizationMethod.RatioToLabel; } } + /// + /// Returns true if this normalization method can be displayed on an axis with a log scale. + /// + public bool AllowLogScale + { + get + { + // "MAXIMUM" and "TOTAL" cannot be displayed on a log scale because they always have to be displayed on a stacked bar plot + // and stacked bar plots do not make sense in a log scale. + return this != MAXIMUM && this != TOTAL; + } + } + public override string ToString() { return Caption; diff --git a/pwiz_tools/Skyline/SkylineGraphs.cs b/pwiz_tools/Skyline/SkylineGraphs.cs index 56b520dda0..0a54168fec 100644 --- a/pwiz_tools/Skyline/SkylineGraphs.cs +++ b/pwiz_tools/Skyline/SkylineGraphs.cs @@ -4455,16 +4455,6 @@ public void SetNormalizationMethod(NormalizeOption normalizeOption) { Settings.Default.AreaNormalizeOption = normalizeOption; SequenceTree.NormalizeOption = normalizeOption; - if (AreaNormalizeOption == NormalizeOption.TOTAL || - AreaNormalizeOption == NormalizeOption.MAXIMUM || - AreaNormalizeOption == NormalizeOption.GLOBAL_STANDARDS || - AreaNormalizeOption.IsRatioToLabel) - { - // Do not let the user combine Log with Ratios because - // the log scale does not work well with numbers that are less than 1. - // (this should be fixed) - Settings.Default.AreaLogScale = false; - } UpdatePeakAreaGraph(); } @@ -4646,8 +4636,10 @@ private void peptideLogScaleContextMenuItem_Click(object sender, EventArgs e) public void ShowPeptideLogScale(bool isChecked) { Settings.Default.AreaLogScale = isChecked ; - if (isChecked) + if (isChecked && !AreaNormalizeOption.AllowLogScale) + { AreaNormalizeOption = NormalizeOption.NONE; + } UpdateSummaryGraphs(); } diff --git a/pwiz_tools/Skyline/TestFunctional/LogScaleAxisTest.cs b/pwiz_tools/Skyline/TestFunctional/LogScaleAxisTest.cs new file mode 100644 index 0000000000..ffd182b2d0 --- /dev/null +++ b/pwiz_tools/Skyline/TestFunctional/LogScaleAxisTest.cs @@ -0,0 +1,162 @@ +/* + * Original author: Nicholas Shulman , + * MacCoss Lab, Department of Genome Sciences, UW + * + * Copyright 2024 University of Washington - Seattle, WA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +using System.Collections.Generic; +using System.Linq; +using DigitalRune.Windows.Docking; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using pwiz.Common.SystemUtil; +using pwiz.Skyline.Controls.Graphs; +using pwiz.Skyline.Model; +using pwiz.Skyline.Model.Results; +using pwiz.SkylineTestUtil; +using ZedGraph; + +namespace pwiz.SkylineTestFunctional +{ + [TestClass] + public class LogScaleAxisTest : AbstractFunctionalTest + { + [TestMethod] + public void TestLogScaleAxis() + { + TestFilesZip = @"TestFunctional\LogScaleAxisTest.zip"; + RunFunctionalTest(); + } + + protected override void DoTest() + { + RunUI(()=> + { + SkylineWindow.OpenFile(TestFilesDir.GetTestPath("LogScaleAxisTest.sky")); + SkylineWindow.ShowPeakAreaReplicateComparison(); + var areaReplicateGraph = FindAreaReplicateGraph(); + Assert.IsNotNull(areaReplicateGraph); + areaReplicateGraph.DockState = DockState.DockBottom; + SkylineWindow.ShowPeakAreaPeptideGraph(); + var areaPeptideGraph = FindAreaPeptideGraph(); + Assert.IsNotNull(areaPeptideGraph); + areaPeptideGraph.DockState = DockState.DockTop; + }); + foreach (var configuration in EnumerateConfigurations(SkylineWindow.Document)) + { + RunUI(() => + { + SkylineWindow.SelectedPath = configuration.IdentityPath; + SkylineWindow.ShowPeptideLogScale(configuration.LogScale); + SkylineWindow.SetNormalizationMethod(configuration.NormalizeOption); + }); + WaitForGraphs(); + RunUI(() => + { + var areaReplicateGraph = FindAreaReplicateGraph(); + Assert.IsNotNull(areaReplicateGraph); + VerifyAxisScale(areaReplicateGraph.GraphControl, "Replicate: " + configuration); + var areaPeptideGraph = FindAreaPeptideGraph(); + Assert.IsNotNull(areaPeptideGraph); + VerifyAxisScale(areaPeptideGraph.GraphControl, "Peptide: " + configuration); + }); + } + } + + private IEnumerable EnumerateConfigurations(SrmDocument document) + { + foreach (var normalizeOption in NormalizeOption.AvailableNormalizeOptions(document)) + { + foreach (bool logScale in new[] { true, false }) + { + foreach (var peptideGroupDocNode in document.MoleculeGroups) + { + foreach (var peptideDocNode in peptideGroupDocNode.Molecules) + { + yield return new Configuration(normalizeOption, + new IdentityPath(peptideGroupDocNode.PeptideGroup, peptideDocNode.Peptide), logScale); + } + } + } + } + } + + private class Configuration + { + public Configuration(NormalizeOption normalizeOption, IdentityPath identityPath, bool logScale) + { + NormalizeOption = normalizeOption; + IdentityPath = identityPath; + LogScale = logScale; + } + + public NormalizeOption NormalizeOption { get; } + public IdentityPath IdentityPath { get; } + public bool LogScale { get; } + + public override string ToString() + { + return string.Format("Normalization:{0} IdentityPath:{1} LogScale:{2}", + NormalizeOption, IdentityPath, LogScale); + } + } + + private void VerifyAxisScale(ZedGraphControl graphControl, string message) + { + for (int iPane = 0; iPane < graphControl.MasterPane.PaneList.Count; iPane++) + { + var graphPane = graphControl.MasterPane.PaneList[iPane]; + VerifyAxisScale(graphPane, message + " Pane#:" + iPane); + } + } + + private void VerifyAxisScale(GraphPane graphPane, string message) + { + var minY = graphPane.YAxis.Scale.Min; + var maxY = graphPane.YAxis.Scale.Max; + + foreach (var curve in graphPane.CurveList) + { + if (curve.IsY2Axis) + { + continue; + } + + var curveMessage = message + " Curve:" + curve.Label.Text; + for (int iPoint = 0; iPoint < curve.NPts; iPoint++) + { + var point = curve.Points[iPoint]; + if (PointPairBase.IsValueInvalid(point.Y)) + { + continue; + } + Assert.IsFalse(point.Y < minY, "{0} should not be less than {1} in point#{2} {3}", point.Y, minY, iPoint, curveMessage); + Assert.IsFalse(point.Y > maxY, "{0} should not be greater than {1} point#{2} {3}", point.Y, maxY, iPoint, curveMessage); + } + } + } + + private GraphSummary FindAreaReplicateGraph() + { + return FormUtil.OpenForms.OfType().FirstOrDefault(graph => + graph.Type == GraphTypeSummary.replicate && graph.Controller is AreaGraphController); + } + + private GraphSummary FindAreaPeptideGraph() + { + return FormUtil.OpenForms.OfType().FirstOrDefault(graph => + graph.Type == GraphTypeSummary.peptide && graph.Controller is AreaGraphController); + } + } +} diff --git a/pwiz_tools/Skyline/TestFunctional/LogScaleAxisTest.zip b/pwiz_tools/Skyline/TestFunctional/LogScaleAxisTest.zip new file mode 100644 index 0000000000..a5c4794be2 Binary files /dev/null and b/pwiz_tools/Skyline/TestFunctional/LogScaleAxisTest.zip differ diff --git a/pwiz_tools/Skyline/TestFunctional/TestFunctional.csproj b/pwiz_tools/Skyline/TestFunctional/TestFunctional.csproj index dc0a729f12..45b6d64628 100644 --- a/pwiz_tools/Skyline/TestFunctional/TestFunctional.csproj +++ b/pwiz_tools/Skyline/TestFunctional/TestFunctional.csproj @@ -205,6 +205,7 @@ +