Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show rdotp in the Replicate comparison peak area window. #1743

Merged
merged 11 commits into from Oct 15, 2021

Conversation

rita-gwen
Copy link
Contributor

No description provided.

@rita-gwen rita-gwen marked this pull request as ready for review September 8, 2021 22:06
@@ -641,6 +651,22 @@ private static void NormalizeGraphToHeavy()
SkylineWindow.UpdatePeakAreaGraph();
}

private void VerifyRdotPLabels(string[] replicates, string[] rdotps)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not going to work right when you run the test in French where numbers are formatted differently. You are going to need to have a list of doubles, and call the method double.ToString(formatString, CultureInfo.CurrentCulture).

var rdotpLabels = SkylineWindow.GraphPeakArea.GraphControl.GraphPane.GraphObjList.OfType<TextObj>().ToList()
.FindAll(txt => txt.Text.StartsWith(@"rdotp")).Select((obj) => obj.Text).ToArray();

Assert.IsTrue(Enumerable.Range(0, replicates.Length).All(i =>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better if you could make this into a for loop which loops over all of the replicates, and calls Assert for each replicate.
It's really hard to debug something like this when it fails because all you get it "Assert.IsTrue failed" and there's no indication of which replicate did it, and what the value was that was being tested.

if (normalizeOption.NormalizationMethod is NormalizationMethod.RatioToLabel ratioToHeavy)
{
var precursorNodePath = DocNodePath.GetNodePath(nodeGroup.Id, document);
if (precursorNodePath.Peptide != null && nodeGroup.LabelType.Equals(IsotopeLabelType.light))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be "!nodeGroup.LabelType.Equals(ratioToHeavy.Label)"
"ratioToHeavy.Label" is the label that is going to appear in the numerator.
If nodeGroup.LabelType is different from ratioToHeavy.Label, then you want to display the rdotp.

@@ -489,7 +495,11 @@ public override void UpdateGraph(bool selectionChanged)
}

// Add area for this transition to each area entry
AddAreasToSums(pointPairList, sumAreas);
Func<double,double, double> aggregateFunc = (sums, y) => sums + y;
if (BarSettings.Type == BarType.Cluster)
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand what this code here is for. If the bar is a cluster then we want to take the Max of all the values instead of summing them? Sounds plausible, but maybe there should be a comment saying why we're doing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is correct. For a cluster graph we need to use max instead of sum.

@@ -320,8 +323,11 @@ public override void UpdateGraph(bool selectionChanged)
if (parentGroupNode.HasLibInfo)
ExpectedVisible = AreaExpectedValue.library;
}
if (ratioToLabel != null && !ratioToLabel.IsotopeLabelType.IsLight && isShowingMsMs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the "!ratioToLabel.IsotopeLabelType.IsLight" here? I would expect that the user could have their normalization either be "ratio to heavy" or "ratio to light", and everything would mostly behave the same but reversed.

@rita-gwen rita-gwen merged commit 8145771 into master Oct 15, 2021
@brendanx67 brendanx67 deleted the Skyline/work/20210826_rdotpInPeakAreaGraph branch January 10, 2022 21:19
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.

None yet

2 participants