Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Added dark-mode diverging colormaps#28587

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
jklymak merged 1 commit intomatplotlib:mainfrommartinosorb:dark-divergent
Aug 1, 2024

Conversation

martinosorb
Copy link
Contributor

PR summary

There is currently no dark-mode diverging colormaps.
This adds three such colormaps,berlin,managua, andvanimo, which have been developed by@Crameri.

Rightfully, there is a resistance against adding new colormaps all the time, however:

  • In issueFeature proposal: "Dark mode" divergent colormaps #18608, there seemed to be a consensus that this would be a sufficiently different type of colormap to justify an addition.
  • They have been developed according to scientific principles, and are meant to be perceptually uniform. The plot in the documentation (reproduced below) shows a better perceptual uniformity than many of the ones we currently have in matplotlib. The author's work principles are illustrated inhis paper.
  • As requested by the author in the Issue discussion, I have kept the same names, and included a reference to the DOI of his work.
  • The "Choosing Colormaps in Matplotlib" user guide has been updated:
    diverging

Closes#18608.

PR checklist

Copy link

@github-actionsgithub-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

For details on testing, writing docs, and our review process, please seethe developer guide

We strive to be a welcoming and open project. Please follow ourCode of Conduct.

@jklymak
Copy link
Member

From the circle log

/home/circleci/project/doc/users/next_whats_new/diverging_colormaps.rst:4: WARNING: py:obj reference target not found: berlin/home/circleci/project/doc/users/next_whats_new/diverging_colormaps.rst:4: WARNING: py:obj reference target not found: managua/home/circleci/project/doc/users/next_whats_new/diverging_colormaps.rst:4: WARNING: py:obj reference target not found: vanimo

I think you just want those as quotes, not back quotes?

[0.71945, 0.9592, 0.60112],
[0.72782, 0.96989, 0.61646],
[0.7362, 0.98063, 0.63191],
[0.74458, 0.99141, 0.64748]]
Copy link
Member

Choose a reason for hiding this comment

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

This sees fine, but are these really defined by a colourable, or do they also have a linear segmented definition? If so, that would be much more compact....

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I guess not, according to the originals...

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I don't know enough to dare changing from the original version, personally.

story645 reacted with thumbs up emoji
Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

Docs need fixing, but otherwise looks OK to me.

Bonus points if you add a section to Choosing Colormaps to get more exposure for this. Not that I agree with everything in Choosing Colormaps, but...

story645 reacted with thumbs up emoji
[0.71945, 0.9592, 0.60112],
[0.72782, 0.96989, 0.61646],
[0.7362, 0.98063, 0.63191],
[0.74458, 0.99141, 0.64748]]
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I guess not, according to the originals...

@martinosorb
Copy link
ContributorAuthor

Thank you for the quick reply.
Ok, I fixed the quotes, let's see if this fixes it.

Regarding Choosing Colormaps, I had added a paragraph saying

  # Berlin, Managua, and Vanimo are dark-mode diverging colormaps, with minimum  # lightness at the center, and maximum at the extremes. These are taken from  # F. Crameri's [scientific colour maps]_ version 8.0.1.

Is that what you meant or did you want something more?

@jklymak
Copy link
Member

Is that what you meant or did you want something more?

I was suggesting adding to:

https://matplotlib.org/stable/users/explain/colors/colormaps.html

which is one of our highest ranking pages for google search.

@martinosorb
Copy link
ContributorAuthor

@jklymak
Copy link
Member

Oh, I'm sorry - I completely spaced thatusers/explain was under galleries now, despite moving it there myself! Anyhow, looks great!

martinosorb reacted with rocket emoji

Copy link
Member

@story645story645 left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Yay, thanks for putting in this PR. Please remove the wrong alt-text. Changing to the right alt-text would be a plus but not strictly necessary for approval.

@martinosorb
Copy link
ContributorAuthor

Sorry for forgetting that. I've now fixed it.

Copy link
Member

@story645story645 left a comment

Choose a reason for hiding this comment

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

Thanks for adding these!

This needs two approvals so pinging@jklymak

@story645
Copy link
Member

story645 commentedJul 19, 2024•
edited
Loading

Also@Crameri does this comply with your requirements in#18608 (comment)?


.. plot::
:include-source: true
:alt: Example figures using "imshow" with dark-mode diverging colormaps on positive and negative data. First panel: "berlin" (blue to red with a black center); second panel: "managua" (orange to cyan with a dark purple center); third panel: "vanimo" (pink to green with a black center).
Copy link
Member

Choose a reason for hiding this comment

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

We don't lint rst files, but pretty sure this should be wrapped. I think as long as you indent the plot directive should still be OK.

Copy link
ContributorAuthor

@martinosorbmartinosorbJul 20, 2024•
edited
Loading

Choose a reason for hiding this comment

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

I wasn't sure whether breaking the line would send me out of thealt variable. I checked the previous what's new files and they seem to go on without breaking the line:https://raw.githubusercontent.com/matplotlib/matplotlib/ed56f95cb6f5b54525687dabbb0e660bafec4d94/doc/users/prev_whats_new/whats_new_3.9.0.rst; so I kept the same style.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure either, and maybe it was done that way on purpose. But if not, then it would be nice to have wrapping

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Unfortunately I was unable to compile the docs locally due to some dependency issues, so I cannot check. Sorry.

Copy link
Member

Choose a reason for hiding this comment

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

Thats why we have CI

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

That didn't work, I reverted.

story645 reacted with thumbs up emoji
@Crameri
Copy link

Also@Crameri does this comply with your requirements in#18608 (comment)?

Yes, perfect!

Thanks all for your initiative and effort on this!

story645 and ingwag reacted with hooray emoji

@story645story645 mentioned this pull requestJul 24, 2024
3 tasks
Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this!

@jklymak
Copy link
Member

Looks like the doc build is failing though, before we merge

@story645
Copy link
Member

Rebase should fix the doc build

@martinosorb
Copy link
ContributorAuthor

I rebased and squashed. CI is green.

ingwag and story645 reacted with thumbs up emojihlongmore reacted with hooray emoji

@jklymakjklymak merged commit27d94d0 intomatplotlib:mainAug 1, 2024
40 checks passed
@jklymak
Copy link
Member

Thanks@martinosorb !

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@story645story645story645 approved these changes

@jklymakjklymakjklymak approved these changes

Assignees
No one assigned
Projects
Milestone
v3.10.0
Development

Successfully merging this pull request may close these issues.

Feature proposal: "Dark mode" divergent colormaps
5 participants
@martinosorb@jklymak@story645@Crameri@QuLogic
[8]ページ先頭
©2009-2025 Movatter.jp