Skip to content

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

Merged
merged 4 commits into from
Apr 30, 2020
Merged

adds turbo colormap #15275

merged 4 commits into from
Apr 30, 2020

Conversation

mikhailov-work
Copy link
Contributor

@mikhailov-work mikhailov-work commented Sep 16, 2019

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

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Copy link
Member

@jklymak jklymak left a 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?

@@ -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
Copy link
Member

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?

Copy link
Contributor Author

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 :)

@@ -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
Copy link
Member

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

Copy link
Contributor Author

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?)

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.

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)

@mikhailov-work
Copy link
Contributor Author

Does colormaps.py require a \n at the end?

@ImportanceOfBeingErnest
Copy link
Member

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 jet in those lists to allow for a better comparisson?

@mikhailov-work
Copy link
Contributor Author

Good idea, did that and added the \n.

@anntzer
Copy link
Contributor

anntzer commented Sep 17, 2019

Actually, how does turbo compare with Peter Kovesi's rainbow, available in colorcet? see https://colorcet.pyviz.org/

@jklymak
Copy link
Member

jklymak commented Sep 17, 2019

The colorcet rainbow doesn't centre on green, which is one of the things that makes jet/turbo useful.

@WeatherGod
Copy link
Member

Just to note something said in a discussion thread:

So since changing the broader profile is outside of scope of 'tweaking', I'm just going to call this one done and leave improvements for a future date :)

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.

@mikhailov-work
Copy link
Contributor Author

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.

@dstansby
Copy link
Member

re. the above two comments, I'll mark this as release critical so we don't forget it for 3.3, but leave it to stew in case any changes want to be made in the next ~6 months.

@dstansby dstansby added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Sep 27, 2019
@tacaswell
Copy link
Member

@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 ;)

@QuLogic QuLogic merged commit 4f42c68 into matplotlib:master Apr 30, 2020
@QuLogic
Copy link
Member

QuLogic commented Apr 30, 2020

Please let us know if there are any issues, the final release isn't out yet.

@story645
Copy link
Member

does this need a what's new entry?

@QuLogic
Copy link
Member

QuLogic commented Apr 30, 2020

Ah, yes, that sounds like a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: color/color & colormaps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants