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

Enh: DivergingNorm Fair #15333

Conversation

ImportanceOfBeingErnest
Copy link
Member

PR Summary

Closes #13948

Adds a new argument fair to the DiverginNorm to be able to create off-centered colormapping with equally spaced colors.

image

While doing this, also removes the restriction vmin <= vcenter <= vmax in DiverginNorm and replace it by vmin <= vmax, such that the center can be outside the interval [vmin, vmax]. This is useful when wanting to show only a partial colormap.

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

@jklymak
Copy link
Member

jklymak commented Sep 24, 2019

I make plots like this all the time, and I just use a linear Norm, and center vmin and vmax around the center, ie for your case with center=0.25, vmin=-0.5, vmax=1.0 and let there be a bunch of blue in the colorbar that doesn’t show up on the plot. I’m not sure what the extra complication gets you....

If this does go in, then I think it should be its own norm, not a kwarg of DivergingNorm.

@ImportanceOfBeingErnest
Copy link
Member Author

For example, if you plot a quantity that cannot be smaller than 0, it doesn't make sense to plot a colorbar with negative values in it.

@jklymak
Copy link
Member

jklymak commented Sep 24, 2019

Theoretically, I suppose someone could want to do that.

Practically, DivergingNorm is for diverging colormaps that usually have a zero color at the centre, and are meant to represent things on either side of zero. It would be strange to represent a positive-definite quantity with a diverging colormap, or have an anomaly that has a hard cap on one side of the center. I’m not saying its impossible someone wants to do this, but its pretty obscure. Certainly if this is to go in, it should be separate from DivergingNorm, which basically does the opposite of what this does.

@ImportanceOfBeingErnest
Copy link
Member Author

It is quite clear that it makes sense to use a DivergingNorm with a Diverging colormap. The usecases are by far not the opposite.

@story645
Copy link
Member

For example, if you plot a quantity that cannot be smaller than 0, it doesn't make sense to plot a colorbar with negative values in it.

Like Jody, I'm unclear on why you'd want to use DivirgingNorm rather than LinearNorm with a one hue colormap. Can you please post an example of your usecase.

I'm also struggling in understanding the 'fair' kwarg-why are you using that term?

@ImportanceOfBeingErnest
Copy link
Member Author

"fair" is just the best I could come up with, it means that both sides of the center are treated equally - or in a fair fashion. "Symmetic" could be an option as well. Native speakers may come up with a better term.

The functionality of fair=True is what seaborn does by default when its center argument is specified.

"LinearNorm with a one hue colormap"

Linear Norm is what @jklymak suggested above, but then you have the problem that you get a useless part of the colorbar which shows the part of the colormap that carries no meaning. Of course one can work around it and create a new colormap for each dataset in use - the idea here is to be able to prevent that.
One-hue colormap or several-hue colormap is not really important here. The important bit is that the center of the normalization is not the center of the colormap. This can sure also make sense with a grey scale colormap (E.g. you have data between 35% and 100%, but want to denote 50% with 50% grey.)

@jklymak
Copy link
Member

jklymak commented Sep 24, 2019

It is quite clear that it makes sense to use a DivergingNorm with a Diverging colormap. The usecases are by far not the opposite.

What I meant by that is DivergingNorm has two different linear ramps to make the endpoints fit inside the colormap, whereas what you are proposing here has just one linear ramp, and basically just truncates the colormap.

I'm not 100% against the idea of this norm, if you really think it will be useful. But I feel it would be far less confusing to have it be its own thing rather than a kwarg of a norm that does something pretty different.

@story645
Copy link
Member

story645 commented Sep 24, 2019

I'm coming around to the idea of this PR if I'm grokking it correctly. I agree with @ImportanceOfBeingErnest that a truncated but equally spaced around a center color colormap can often be the correct mapping for a DivergingNorm.

I think I even have a use case from this summer: mapping errors where they skewed so extreme in one direction that putting both sides on the cmap would end up with a ton of wasted space (it was like a 10 to -1 spread) that would more accurately be conveyed with the proposed cmap.

What about 'equally_spaced' as the kwarg?

@jklymak
Copy link
Member

jklymak commented Sep 24, 2019

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.colors import DivergingNorm

np.random.seed(19680801)
data = np.random.rand(4, 11) - 0.25

fig, (ax1, ax2) = plt.subplots(ncols=2, figsize=(7, 2.5), constrained_layout=True)

norm1 = DivergingNorm(0.0, vmin=-0.25, vmax=0.75, fair=True)
im = ax1.imshow(data, cmap='RdBu_r', norm=norm1)
cbar = fig.colorbar(im, ax=ax1, orientation="horizontal", aspect=15)

im = ax2.imshow(data, cmap='RdBu_r', vmin=-0.75, vmax=0.75)
cbar = fig.colorbar(im, ax=ax2, orientation="horizontal", aspect=15)

plt.show()

TestFair

I find the second plot far easier to parse. The zero on the colorbar is in the middle, and its clear that a similar conceptual scale has been used on each side of the zero. The "wasted" space in the colorbar doesn't bother me in the least, indeed it makes it crystal clear that the anomaly from zero is much more positive than negative.

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.

Please make its own Norm subclass.

@@ -1061,11 +1061,11 @@ def scaled(self):


class DivergingNorm(Normalize):
def __init__(self, vcenter, vmin=None, vmax=None):
def __init__(self, vcenter, vmin=None, vmax=None, fair=False):
Copy link
Member

Choose a reason for hiding this comment

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

I'll block on this being kwarg. Skptical about the normalization on the whole, but very against it being a kwarg for DivergingNorm

Copy link
Member

@story645 story645 Sep 24, 2019

Choose a reason for hiding this comment

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

But I think this belongs in DivergingNorm since that's the use case for truncated around a center colormap. Can we pitch this to the call? (which I'm probably going to be late to).

Copy link
Member

Choose a reason for hiding this comment

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

From a user point of view norm1 = OffcenterNorm(0.0, vmin=-0.25, vmax=0.75) is easier to type, and can get its own section in the docs, rather than making the docs and code for DivergingNorm have a confusing kwarg that does something quite different from what fair=False does. I don't understand the desire to make this a kwarg.

@ImportanceOfBeingErnest
Copy link
Member Author

ImportanceOfBeingErnest commented Sep 24, 2019

The reasons I put this in DivergingNorm are:

  • In my eyes both are conceptually similar, with "diverging" meaning diverging from a center point. With this there are two options, one which was implemented beforehands, namely to squeeze everything into the range [0,1], the other, implemented here, to let it expand past that range but with equal (or fair) rate of divergence.
    With this in mind, I think it's natural for a user to look for this functionality under the name DivergingNorm.

  • Since the code is mostly the same in both cases, I didn't feel like it makes sense to create yet another instance of Norm; but instead keep everything in one place. This being said, one could of course create a new norm, subclassing the old one

    class DivergingNormFair(DivergingNorm):
        def __init__(vcenter, vmin=None, vmax=None):
            super().__init__(vcenter, vmin, vmax, fair=True)
    

@tacaswell
Copy link
Member

This ended up being a point of discussion on the call this week, see https://paper.dropbox.com/doc/In-Matplotlib-2019-meeting-agenda--AlZNZ8evWRf18Do4TtHUpBxuAg-aAmENlkgepgsMeDZtlsYu#:h2=PRs

The short version is that this is pushing on important functionality, but we are not sure what the correct API.

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

This needs further discussion and possibly an API re-think / extension.

@efiring
Copy link
Member

efiring commented Sep 24, 2019

Based on discussion in the Sept. 24, 2019 call, this is a more complicated issue than it appears, and needs some higher-level API design proposals. The consensus is that although aspects of this PR might end up being adopted, the PR as it stands should not be merged.

@pharshalp
Copy link
Contributor

as a user, I can definitely vouch for a use case for this kind of norm. I have been using a similar functionality for showing relative sound levels (+- some critical value which is not necessarily zero) across a 2D spatial grid.

@timhoffm
Copy link
Member

@jklymak
Copy link
Member

jklymak commented Sep 24, 2019

as a user, I can definitely vouch for a use case for this kind of norm. I have been using a similar functionality for showing relative sound levels (+- some critical value which is not necessarily zero) across a 2D spatial grid.

Thats understandable, but why would you want to truncate the colormap to plot that? You can just do: vmin=crit - range vmax = crit+range and adjust range to suit...

@pharshalp
Copy link
Contributor

pharshalp commented Sep 24, 2019 via email

@jklymak
Copy link
Member

jklymak commented Sep 24, 2019

I think the consensus was that this PR is probably the correct approach (plus or minus) but a) definitely should be its own norm, and b) maybe there is a better way to do this? i.e. change the limits of the colorer directly, or alter the colormap directly. Having a norm that maps vmin to something other than 0 was a stumbling block.

@pharshalp
Copy link
Contributor

pharshalp commented Sep 24, 2019 via email

@jklymak
Copy link
Member

jklymak commented Sep 25, 2019

As is, this isn't behaving quite correctly if the values are outside of vmin/vmax?

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.colors import DivergingNorm

np.random.seed(19680801)
data = np.random.rand(10, 11)*2. - 1

fig, (ax1, ax2) = plt.subplots(ncols=2, figsize=(7, 2.5), constrained_layout=True)

norm1 = DivergingNorm(0.0, vmin=-0.1, vmax=0.75, fair=True)
im = ax1.imshow(data, cmap='RdBu_r', norm=norm1)
cbar = fig.colorbar(im, ax=ax1, orientation="horizontal", aspect=15, extend='both')

im = ax2.imshow(data, cmap='RdBu_r', vmin=-1, vmax=1)
cbar = fig.colorbar(im, ax=ax2, orientation="horizontal", aspect=15, extend='both')

Figure_1

@Tillsten
Copy link
Contributor

I also want to supported this feature: I am working with difference spectra, hence the special value is 0. So far I just set vmin and vmax as proposed, but this leads to a misleading colorbar, since it indicates the occurrence of values outside the observed range. There is also no working way to set the limits of the colorbar.

@ImportanceOfBeingErnest
Copy link
Member Author

As is, this isn't behaving quite correctly if the values are outside of vmin/vmax?

Indeed the question I wasn't sure of how to handle is this: Should a norm always map to [0, 1]? Or should it be restricted to a smaller range?

@ImportanceOfBeingErnest
Copy link
Member Author

I fixed the example from above. It would now look like this:

image

meaning everything outside [vmin,vmax] is mapped to the under/over color.

This wasn't even the case for DivergingNorm so far. E.g.

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.colors import DivergingNorm

np.random.seed(19680801)
data = (np.random.rand(4, 11)-0.5)*3

cmap=plt.get_cmap("RdBu")
cmap.set_under("yellow")

fig, ax = plt.subplots(figsize=(5, 2))

norm1 = DivergingNorm(0.25, vmin=0, vmax=1)
im = ax.imshow(data, cmap=cmap, norm=norm1)
cbar = fig.colorbar(im, ax=ax, orientation="horizontal", aspect=15, extend='both')

plt.show()

looks like this, where outside values are just mapped to the lowest color from the colormap, not the under/over colors.

image

In the current state of this PR it will now look as follows, as expected.

image

@jklymak
Copy link
Member

jklymak commented Sep 26, 2019

Agreed that DivergingNorm as it now stands does not have over/under support, largely because the person who wrote it forgot about that feature of colormaps.

@pharshalp
Copy link
Contributor

pharshalp commented Oct 1, 2019

If the DivergingNorm is replaced by TwoSlopesNorm and the norm in this PR is decided to be a separate norm, then would it be sensible to call it TwoSlopesFairColorsNorm?

@jklymak
Copy link
Member

jklymak commented Oct 1, 2019

I think we should leave Fair out of the name. I don't see where "fairness" comes into the picture 😉

@pharshalp
Copy link
Contributor

TwoSlopesProportionalColorsNorm?

@jklymak
Copy link
Member

jklymak commented Oct 1, 2019

Its not a two-slope norm!

@Tillsten
Copy link
Contributor

Tillsten commented Oct 1, 2019

If you really want to change the name I would propose to call it MidpointNorm and add the fair option to it. Because I quite sure that setting the midpoint is what most people want to do.

@jklymak
Copy link
Member

jklymak commented Oct 1, 2019

  1. yes we will change the name of DivergingNorm (DivergingNorm is a misleading name #15336 for name discussions)
  2. no, we will not have a kwarg for DivergingNorm that provides the functionality described here; this norm needs its own name.

@ImportanceOfBeingErnest
Copy link
Member Author

I do not understand the necessity of renaming DivergingNorm. I also do not understand the reason the solution here needs its own class. I feel that the reasoning behind this has not been well articulated so far. (And just stating something as a fact is not helpful in such case.) In total I have the feeling that all of this creates much more trouble (for devs, as well as for users) than just not renaming it.

But given that there seems to be a consensus among several devs, what are the consquences for this PR:

If DivergingNorm is renamed to TwoSlopeNorm it means that the "fair" Norm that is proposed here
a. cannot take over the TwoSlope part, because it simply has no two slopes.
b. cannot continue to be named DivergingNorm, because that would clash with the deprecation of the original DivergingNorm
c. should not be named by any name that has previously been publically used for what is now the DivergingNorm. This means in particular, it should not be named MidPointNorm, MidpointNormalize or PiecewiseLinearNorm. The reason is that it would surely clash with what people who have been using any of those names in the past would be expecting from it.

Consequently, a new name would be needed. The word "fair" also seems to cause misinterpretation, so let's not use it. Possibly CenterNorm, CenteredNorm or similar might make sense?

In addition, @timhoffm proposed a broader picture of the usecase in https://discourse.matplotlib.org/t/limiting-colormapping/20598. So one might take into account that people would, instead of defining a center point, want to define the two edges of the normalization region. This could in principle be combined into the same normalization instance, with an API like

SomeNorm(vcenter=None, vmin=None, vmax=None, clip_min=None, clip_max=None)

with all parameters being optional and, if not given, determined by autoscaling to the data.

@jklymak
Copy link
Member

jklymak commented Oct 2, 2019

I think something like ClippedNorm and CenteredClippedNorm would make sense to me. You are basically clipping the usual 0-1 result of the mapping to a new range a to b, where 0<=a<=b<=1.

WRT to not making this a kwarg of DivergingNorm, its is just confusing from a documentation point of view: DivergingNorm does this mapping, except with this kwarg, in which case it does a very different mapping. All the norms could be kwargs of Normalize in that case.

@jklymak
Copy link
Member

jklymak commented May 8, 2021

I'm going to close this. I'm not really convinced its a good idea, but if someone wants to take it up they can re-base etc in a new PR.

@jklymak jklymak closed this May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow colorbar.ax.set_ylim to set the colorbar limits?
9 participants