-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
adds turbo colormap #15275
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
adds turbo colormap #15275
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great except for some formatting issues?
lib/matplotlib/pyplot.py
Outdated
@@ -1943,6 +1943,9 @@ def colormaps(): | |||
originally from the Neuroimaging in Python project | |||
terrain mapmaker's colors, blue-green-yellow-brown-white, | |||
originally from IGOR Pro | |||
turbo a spectral map (purple-blue-green-yellow-orange-red) with | |||
a bright center and darker endpoints. a smoother |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix the capitalization and indenting here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Sorry capitalization style wasn't clear from the above :)
tutorials/colors/colormaps.py
Outdated
@@ -349,7 +351,7 @@ def plot_color_gradients(cmap_category, cmap_list, nrows): | |||
# overlaid, labeled contours could help differentiate between one side of the | |||
# colormap vs. the other since color cannot be used once a plot is printed to | |||
# grayscale. Many of the Qualitative and Miscellaneous colormaps, such as | |||
# Accent, hsv, and jet, change from darker to lighter and back to darker gray | |||
# Accent, hsv, jet and turbo, change from darker to lighter and back to darker gray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a 79-character limit, so this won't pass our format requirement test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it up, though some lines above are at 80 chars, just fyi... (should I fix those too?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this file is excluded from the 80 char limit.
Line 53 in a813b92
tutorials/colors/colormaps.py: E501 |
This means you can either leave it as it is or fix all of them and remove the entry in the .flake8
file. (One should be aware that the more changes are made at different points, the higher the chances one will later on need to rebase; this might speak in favour of doing those line-length changes in a separate PR)
Does colormaps.py require a \n at the end? |
Yes. And while everyone's in the nitpicky mode here: Would it make sense to put turbo directly below |
Good idea, did that and added the \n. |
Actually, how does turbo compare with Peter Kovesi's rainbow, available in colorcet? see https://colorcet.pyviz.org/ |
The colorcet |
Just to note something said in a discussion thread:
As a policy, Matplotlib does not make changes to released colormaps. While you would have until the v3.3 release to make any changes you feel is necessary, this is really why I have always felt that a publication requirement was important for colormaps. The viridis colormap went through a lot of vetting and modifications before it was even named (it was originally called "option D", if I remember correctly). The Brewer colormaps were all published elsewhere before getting included in matplotlib. A few others were all tweaked and modified as part of a peer review process prior to publication that finalized them. If you want to do major changes to the colormap, then I would suggest doing so and going through the publication route again. Or, perhaps consider calling such a modified colormap a new colormap with a new name (and again, go through a publication route). But don't expect to be able to make any further changes to Turbo in this package once it is in a matplotlib release. |
Understood. I think we're actually saying the same thing, because I agree. I basically was trying to say that small tweaks to address people's feedback didn't work, and big changes would mean essentially a new colormap, new publication etc, so let's add turbo as is with it's strengths+weaknesses, and not confuse the matters. If someone comes up with (substantial) improvements to this, it'll likely look different enough for a new publication, discussion, and so on. |
re. the above two comments, I'll mark this as release critical so we don't forget it for |
@mikhailov-work Have there been any updates to turbo? I am going to rebase and force-push so it will merge cleanly. We are getting close to the 3.3 release so we are getting close to the point of no return ;) |
- moving turbo to be near jet for better comparison
Please let us know if there are any issues, the final release isn't out yet. |
does this need a what's new entry? |
Ah, yes, that sounds like a good idea. |
PR Summary
This PR adds the Turbo colormap, originally proposed in
https://ai.googleblog.com/2019/08/turbo-improved-rainbow-colormap-for.html
Discussion is found in #15091
PR Checklist