Skip to content

Diverging norm #5054

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

Closed
wants to merge 16 commits into from
Closed

Diverging norm #5054

wants to merge 16 commits into from

Conversation

jkseppan
Copy link
Member

Renamed and rebased version of #4666

phobson and others added 16 commits September 13, 2015 15:13
Borrows heavily from @Tillsen's solution found on
StackOverflow here: http://goo.gl/RPXMYB

Used with his permission dicussesd on Github here:
https://github.com/matplotlib/matplotlib/pull/3858`
This will allow the ticks of colors to be spaced as desired.
Also simplified the math per the brilliant @joferkington
http://stackoverflow.com/a/20146989/1552748
couldn't actually run the test suite b/c python
iesn't installed as a framework.
Need to allow it in __call__ since colorbar can pass a value for clip.
To make it more similar to the other norms

Also remove misleading comment about returning scalars
Since possibly some earlier versions of numpy returned a scalar,
wrap the value in atleast_1d before indexing.
np.interp handles the case vmin == vcenter, we have to add a special case
to make the last test case pass
assert_array_almost_equal(normed_vals, self.expected)

def test_inverse(self):
if self.test_inverse:
Copy link
Member

Choose a reason for hiding this comment

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

I am a bit confused about how I expect this to behave...

@tacaswell
Copy link
Member

I am a bit concerned about the inverse tests:

In [14]: nn = mc.DivergingNorm(0, 1, 10)

In [15]: nn.inverse(nn(np.arange(10))) == np.arange(10)
Out[15]: 
masked_array(data = [ True False False False False False False False False False],
             mask = False,
       fill_value = True)

@tacaswell
Copy link
Member

If I add the inverse function back in the color bar does not work correctly even with spacing=proportional

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.colors as mcolors

x = np.linspace(-2, 7)
y = np.linspace(-1*np.pi, np.pi)
X, Y = np.meshgrid(x, y)
Z = x * np.sin(Y)**2

fig, (ax1, ax2, ax3) = plt.subplots(ncols=3, figsize=(9, 3))
cmap = plt.cm.coolwarm
norm = mcolors.DivergingNorm(vmin=-2, vcenter=0, vmax=7)

img1 = ax1.imshow(Z, cmap=cmap, norm=None)
cbar1 = fig.colorbar(img1, ax=ax1)

img2 = ax2.imshow(Z, cmap=cmap, norm=norm)
cbar2 = fig.colorbar(img2, ax=ax2)

img3 = ax3.imshow(Z, cmap=cmap, norm=norm)
cbar3 = fig.colorbar(img3, ax=ax3, spacing='proportional')

gives
so

attn @efiring

    def inverse(self, value):

        result, is_scalar = self.process_value(value)

        self.autoscale_None(result)
        vmin, vcenter, vmax = self.vmin, self.vcenter, self.vmax
        if vmin == vmax == vcenter:
            result.fill(0)
        elif not vmin <= vcenter <= vmax:
            raise ValueError("minvalue must be less than or equal to "
                             "centervalue which must be less than or "
                             "equal to maxvalue")
        else:
            vmin = float(vmin)
            vcenter = float(vcenter)
            vmax = float(vmax)
            # in degenerate cases, prefer the center value to the extremes
            degen = (result == vcenter) if vcenter == vmax else None

            x, y = [vmin, vcenter, vmax], [0, 0.5, 1]
            result = ma.masked_array(np.interp(result, y, x),
                                     mask=ma.getmask(result))
            if degen is not None:
                result[degen] = 0.5

        if is_scalar:
            result = np.atleast_1d(result)[0]
        return result

@tacaswell
Copy link
Member

@efiring I have sunk another few hours into trying to understand the color bar code and am still failing 😞 (but I am getting closer!). It looks like 'proportional' is intended for use with the BoundayNorm and not much else?

@tacaswell
Copy link
Member

I think the problem is that for 'proportional' when the color bar is trying to guess what the correct value for b should be in _process_value it should not use the inverse, but I can not figure out what it should use instead. I tried just using np.linspace(vmin, vmax), but that doesn't work right (I think there is someplace else in the ticking where it gets compensated for?) and it breaks LogNorm with spacing='proportional'.

@efiring
Copy link
Member

efiring commented Sep 14, 2015

@tacaswell, the use of inverse is deliberate; I found I needed it when I added LogNorm. It allows one to find the boundaries in data space corresponding to a linear increase in plot space. This is critical. I'm puzzled as to why the inverse inherited from Normalize seems to be giving a reasonable result. Please give me a little time to figure this out. I agree that the colorbar code is very hard to follow, even for me, who wrote most of it many years ago.
Regarding the spacing kwarg: it is intended to make a difference for "discrete colors", as it says in the docstring--but that could be made clearer. This includes BoundaryNorm, and the discrete boundaries from contourf. It seems like it could also make a difference in the ordinary, quasi-continuous case, and I don't remember whether it ever did, or whether it intentionally does not.

@efiring
Copy link
Member

efiring commented Sep 14, 2015

Looking at #5061, I think I understand: the picture you are showing is with the inverse, and #5061 is showing the picture without the inverse. Well, the picture with the inverse is correct in the sense that it is consistent with colorbar's logic--it just isn't what you expect or want. Consider the case of a more extreme norm, the LogNorm: if you were to put in linearly-selected tick values, they would be all bunched together at one end.
I'm still not sure what the solution is; I will look again tomorrow.

@OceanWolf
Copy link
Member

Hmm, yes, colorbar code, I took a look at it last night when working on the PiecewiseLinearNorm in #5061, it should serve as an alternative to this PR as DivergingNorm subclasses PiecewiseLinearNorm.

@efiring @tacaswell you should feel happy to note that I have a plan on refactoring the colorbar. While the code still lay clear in my mind of the parts of colorbar that I looked at (the last else bit in ColorbarBase_process_values, and updated MEP29 (Axes refactor) with the new information I gleaned there, still very much a W-I-P so no need to look at it now, just to let you know it does and will get some attention in the near future 👼.

@tacaswell tacaswell added this to the next point release milestone Sep 14, 2015
@tacaswell
Copy link
Member

I am starting to think that getting this in for 1.5 is a bigger can of worms than anticipated.

@tacaswell tacaswell modified the milestones: next point release, proposed next point release Sep 14, 2015
@tacaswell tacaswell removed this from the next point release milestone Sep 14, 2015
@tacaswell
Copy link
Member

Due to the issues with the inverse and the colorbar, punting this to the next release.

@efiring
Copy link
Member

efiring commented Sep 14, 2015

On 2015/09/14 5:50 AM, Thomas A Caswell wrote:

Due to the issues with the inverse and the colorbar, punting this to the
next release.

Good move. I don't see it as urgent.

@OceanWolf
Copy link
Member

Okay, I now understand the inverse thing in ColorbarBase and I document this in MEP29, as I see it we have two different scales going on, the scale used by Normalize to map the values to a colour, and the scale used to draw the colorbar. ColorbarBase currently assumes this later scale as the inverse of that given by the Normalize class, i.e. with the inverse, when we specify a center value for example, 0.5, in the example above vcenter=0.5, this means that the colorbar will currently obey that.

So the solution lies into seperating this conflation of scales into the two distinct scales, something I had planned for MEP29, but didn't realise until now just how significantly this refactor plays into this issue.

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017
@jklymak
Copy link
Member

jklymak commented May 9, 2018

This seems to have died an untimely death. Does anyone want to champion?

@OceanWolf
Copy link
Member

How about making such thing a wishlist item or something similar, to make it something that can get picked up, but doesn't count as a blocker for release.

@jklymak jklymak added Good first issue Open a pull request against these issues if there are no active ones! status: needs rebase New feature and removed status: needs review labels May 17, 2018
@jklymak jklymak modified the milestones: needs sorting, v3.0 May 17, 2018
@jklymak jklymak modified the milestones: v3.0, v3.1 Jul 9, 2018
@jklymak
Copy link
Member

jklymak commented Oct 6, 2018

Moved to #12419....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Open a pull request against these issues if there are no active ones! New feature status: needs rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants