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

Improvements to LogScale in bar graphs #3284

Merged
merged 13 commits into from
Dec 28, 2024

Conversation

nickshulman
Copy link
Contributor

  1. Correct the Y-axis range when displaying values that are less than 1.
    image
    "CurveList.GetRange" used to think that all bar graphs should include the number 0. That was change to not do that if the axis is a log scale.
    Values which would be zero are replaced with PointPair.Missing
    Removed the code which did not allow you have a log scale with normalization methods such as "ratio to global standards" or "ratio to heavy" which would often have values that were less than 1.
    When normalized to "Total" or "Maximum", log scale is still forbidden.

  2. Always switch to BarType.Cluster if the Y-axis is log scale
    image
    This is because stacked bar graphs with a log scale give a very misleading idea of which bar in the stack is the biggest.
    Also, it would be tricky to figure out what the range of the Y-axis should be.

  3. This also works for the Peak Area Peptide Comparison
    image

I will try to write a unit test which verifies that the selection box is drawn in the right place and the Y-axis range is what we would expect.

@brendanx67
Copy link
Contributor

Looks much better. Though, I wonder in the first 2 cases why it 10^-1. Seems like it is adding a lot of unnecessary white space to the graph.

GraphHelper.ReformatYAxis was using Scale.MajorStep which does not have a meaningful number when the axis is a log scale.
@nickshulman
Copy link
Contributor Author

Though, I wonder in the first 2 cases why it 10^-1. Seems like it is adding a lot of unnecessary white space to the graph.

Good point. "GraphHelper.ReformatYAxis" was doing the wrong thing when the Y-axis scale was log.
It was incorrectly ensuring that the number 1 (i.e. 10^0) was included because that was the value of Scale.MajorStep.

image

@nickshulman nickshulman merged commit 9d35570 into master Dec 28, 2024
11 checks passed
@nickshulman nickshulman deleted the Skyline/work/20241217_LogScaleFixes branch December 28, 2024 00:31
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.

2 participants