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

Fix colorbar orientation to always be min->max #27

Merged
merged 7 commits into from
Feb 15, 2023

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jan 31, 2023

A user of one of my projects noticed that the colors for vertical colorbars (scales) had the colors flipped from what they should be. As far as I can tell this has always been like this. Additionally, the colormap was always assumed to have increasing values, but in recent versions of trollimage it is possible to have values in reverse order (flipped values versus flipped colors). This produces colorbars that don't follow what I think most people would expect which is for the colorbar to go from the minimum value to the maximum value. In the horizontal orientation this means minimum value on the left, maximum on the right. For vertical orientation this means minimum value on the bottom, maximum on the top. This PR fixes all of this.

montage

@djhoese djhoese added the bug label Jan 31, 2023
@djhoese djhoese requested review from mraspaud and pnuu January 31, 2023 15:29
@djhoese djhoese self-assigned this Jan 31, 2023
@djhoese
Copy link
Member Author

djhoese commented Jan 31, 2023

Oh I also added pre-commit.ci to this repository. We were already using pre-commit-config in the project, but I forgot to enable the repository on pre-commit.ci.

@djhoese
Copy link
Member Author

djhoese commented Jan 31, 2023

Ok I've updated the original description with example images. These are using the "rdbu" colormap so the first color is red, the last color is blue.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4055840102

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 3685681590: 0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jan 31, 2023

Pull Request Test Coverage Report for Build 4185617677

  • 57 of 57 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.4%) to 66.016%

Totals Coverage Status
Change from base Build 4175923890: 3.4%
Covered Lines: 338
Relevant Lines: 512

💛 - Coveralls

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM, Just a suggestion to use linspace instead of arange

pydecorate/decorator_base.py Outdated Show resolved Hide resolved
pydecorate/decorator_base.py Outdated Show resolved Hide resolved
@djhoese
Copy link
Member Author

djhoese commented Feb 15, 2023

As mentioned on slack, here is what this particular colormap looks like with the new HCL logic in trollimage:

image

About the same in my opinion, but maybe a little smoother/cleaner on the red to white transition.

@djhoese djhoese merged commit c50bcc8 into pytroll:main Feb 15, 2023
@djhoese djhoese deleted the bugfix-vertical-cbar branch February 15, 2023 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants