matplotlib / cmocean

Colormap setup for standardizing commonly-plotting oceanographic variables.
MIT License
230 stars 52 forks source link

Adds functionality for inverting colormap lightness #106

Closed andrew-s28 closed 6 months ago

andrew-s28 commented 8 months ago

I went with the option of adding the function invert_lightness to cmocean.tools, since having more than just an _r subscript on colormaps would be confusing.

This solution does require adding colorspacious as a dependency to cmocean.

Closes #105

Here is an example of the inverted lightness colormaps: invert_lightness_example

kthyng commented 8 months ago

I see this! Hope to get to it today. 🌈

kthyng commented 8 months ago

🌈 as a happy symbol of course, not as an implicit endorsement of using rainbow colormaps.

kthyng commented 8 months ago

@andrew-s28 I realize now I didn't give feedback on this in your original issue, but I'm interested in having the "_i" or "_inverted" access to these colormaps. How hard would this be to implement, and would it take much time to calculate on the fly if a user called it? I think it would be much more user-friendly since I doubt that many users of cmocean use the tools at all. Though I do appreciate your concern it might be confusing to have more underscore-based names, especially since I don't know of any more conventions besides "_r".

andrew-s28 commented 8 months ago

@kthyng Thanks for the review!

It would be straightforward to add access to the inverted colormaps via an _i appendix instead of the way it is now. I could implement it such that order of the appendix does not matter (whether _r or _i comes first). There is very little overhead to calling the inversion on the fly, so I think it would be fine to implement it such that whatever colormap the user calls is passed to the inverting function when they call it.

An alternative would be to include the inverted colormaps into -rgb.txt files, similar to how the existing colormaps are implemented. This option does have the advantage that the inverted colormaps would be generated ahead of time and thus would not require the addition of the colorspacious dependency.

Let me know which way you think would be best.

kthyng commented 8 months ago

An alternative would be to include the inverted colormaps into -rgb.txt files, similar to how the existing colormaps are implemented

Ah, yes, people always like to limit the dependencies. Also many other packages have included the rgb files directly so this would have the advantage of being able to pass them forward if anyone is interested. I would lean this way even if it seems a bit more brute force. What do you think?

andrew-s28 commented 8 months ago

I admit to being one of those people - I tend to be wary of suggesting additional dependencies, especially to a well-established package and since colorspacious doesn't seem particularly well maintained these days.

I actually prefer the brute force solution myself as well, it also prevents any future maintenance issues with the additional function in cmocean.tools. My only hesitation was the extra _i appendix that could be confusing, but since you don't think that will be a problem, I can go ahead and put together the -rgb.txt files for the inverted colormaps and submit a new PR that allows them to be called directly from cmocean.cm.

kthyng commented 8 months ago

@andrew-s28 Yes I think that with some good documentation (is that in this same repo? it's been awhile since I updated that and I wrote all this a long time ago) and also maybe multiple aliases this will be clear enough. What about making available both or all *_inv or *_invert and *_i, whatever seems like it would capture a user's tendencies?

andrew-s28 commented 8 months ago

@kthyng Seems like a reasonable plan to me, I think I will implement it such that if a _i is present in the colormap then it will return an inverted colormap (this would cover all of the potential cases such as _inv and _inverted as well). Should be able to get that done early next week, since everything is already set up to produce the -rbg.txt files.

The documentation is in this repo, so I will add a short description of inverted colormaps to the docs with this PR as well.

andrew-s28 commented 7 months ago

@kthyng I have updated the PR to used hard-coded rgb files for the inverted colormaps, accessible with a *_i appendix in the same way current colormaps are. Reversed and inverted colormaps are available with a *_r_i or *_i_r appendix (order doesn't matter).

Documentation is added right after the documentation for reversed colormaps.

I also made a few changes to the cmocean.plots module so that plotting functions such as plots.plot_lightness() and plots.plot_gallery() are unchanged with the extra entries in cmap_d.

kthyng commented 6 months ago

@andrew-s28 Ack sorry this came in at a particularly busy time and then moved down the email list. I'll get to it today or tomorrow I think.

kthyng commented 6 months ago

@andrew-s28 This looks really great!! I'm going to accept and then will process it on through.