Skip to content

QuadMesh.get_clim changed behavior in 3.3.0rc1 #17703

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
MaozGelbart opened this issue Jun 21, 2020 · 5 comments · Fixed by #17859
Closed

QuadMesh.get_clim changed behavior in 3.3.0rc1 #17703

MaozGelbart opened this issue Jun 21, 2020 · 5 comments · Fixed by #17859

Comments

@MaozGelbart
Copy link
Contributor

Bug report

Bug summary

QuadMesh.get_clim() is documented as:

Return the values (min, max) that are mapped to the colormap limits.

Without vmin or vmax arguments to pcolormesh, this returns the data minimal and maximal values. However in the presence of only one of vmin or vmax, a change occurred between 3.2.2 and 3.3.0rc1. In 3.3.0rc1 it returns the input vmin and vmax, though only one of them was specified. The provided examples also happens (with slight output changes) for setting vmax instead of vmin.

Code for reproduction

import numpy as np
import matplotlib.pyplot as plt

dx, dy = 0.15, 0.05
y, x = np.mgrid[slice(-3, 3 + dy, dy),
                slice(-3, 3 + dx, dx)]
z = (1 - x / 2. + x ** 5 + y ** 3) * np.exp(-x ** 2 - y ** 2)
z = z[:-1, :-1]

f, ax = plt.subplots(1,1)
ax.pcolormesh(x, y, z, vmin=0)
ax.axis([x.min(), x.max(), y.min(), y.max()])
for mesh in ax.collections:
    print(mesh.get_clim())

Actual outcome

# 3.3.0rc1
(0.0, None)

Expected outcome

# 3.2.2
(0.0, 1.0510083319982622)

Matplotlib version

  • Operating system: MacOSX
  • Matplotlib version: 3.3.0rc1
  • Matplotlib backend (print(matplotlib.get_backend())): MacOSX
  • Python version: 3.8.3

Installed using pip

@jklymak
Copy link
Member

jklymak commented Jun 22, 2020

I'm not sure if this behaviour is expected new behaviour or not. 0a2f082 is the commit (@timhoffm)
#15769

@tacaswell
Copy link
Member

Ah, it looks like in some cases we either called set_clim() or called autoscale_none but in some cases we did both. #15769 standardized everything to always do or. The autoscaling will eventually get taken care of down stream during the draw process.

Adding mesh.autoscale_None() before checking the limits fixes the old behavior (without forcing a full draw).

I don't see a way to put a deprecation warning on this behavior without being too annoying so I think out options are

  1. document this as a hard break with the work-around of calling autoscale_None if you need to get the clim before drawing
  2. add back the special casing from pcolor and pcolormesh and accept the slight difference in behavior between the artists
  3. change behavior of everything else and make _scale_norm always call autoscale_None (which is an API change everywhere else)

While writing this out I am various points had convinced my self that all 3 options were the "best" option ;)

1 / 3 get us consistency but break different directions on if we should resolve the "auto" limit as soon as possible or as late as possible

2 gets us a little change in behavior as possible, but means that the early/late resolution varies by plotting method / Aritst.

@tacaswell
Copy link
Member

And thank you @MaozGelbart for testing the RC :)

@tacaswell
Copy link
Member

We discussed this on the call and noted that if you pass nothing we call autoscale_None at init time. I am going to write up a set of example to make sure we fully understand what it is going on, when various things are called.

@tacaswell
Copy link
Member

This turned into a rabbit hole of complexity.

This is the current state for everything on 3.3

  • pass no norm / vmin / vmax -> call autoscale_None on data passed in immediately
  • pass norm -> call autoscale_None on data passed in immediately
  • pass vmin or vmax -> defer calling autoscale_None until the norm actually need to be used
  • pass vmin and vmax -> doesn't matter because nothing to autoscale

I am more convinced that we want to change it so in all cases when the Axes methods return we have fully resolved the vmin/vmax based on the input of {norm, vmin, vmax, data}, but we will need to document this as an intentional API change. This is going to break people who

a) pass in dummy data + one of vmin or vmax
b) re-set the data in the artist before the first time the norm is used (via drawing, adding a colorbar, direct usage...)

I am hoping this is rare because it seems like it is quite brittle.


This is what I was using to play with this:

import functools

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

class ForgivingNorm(mcolors.Normalize):
    @functools.wraps(mcolors.Normalize.__call__)
    def __call__(self, value, *args, **kwargs):
        try:
            return super().__call__(value, *args, **kwargs)
        except ValueError:
            if self.vmin < self.vmax:
                # not sure why we are here?!
                raise
            # we are now here because self.vmax < self.vmin
            # normally this is something we raise an exception and kill
            # the draw on, but we want this to work 
            result, is_scalar = self.process_value(value)
            result = np.ma.array(result.data, mask=True, copy=False)
            if is_scalar:
                result = result[0]
            return result

    def __repr__(self):
        return f'<ForgivingNorm(vmin={self.vmin}, vmax={self.vmax})>'

# monkey patch for reasons
mcolors.Normalize = ForgivingNorm

zero_data = np.zeros((5, 5))

ramp_data = np.arange(25).reshape(5, 5) + .5




def run_test(zero_data, ramp_data, *, early_autoscale_None, make_norm):
    fig, ax_arr = plt.subplots(2, 2, squeeze=False)

    [[ax1, ax2], [ax3, ax4]] = ax_arr

    im_dict = {}

    for j, (ax, kwargs) in enumerate(
        zip(ax_arr.ravel(), ({}, {"vmin": 10}, {"vmax": 15}, {"vmin": 10, "vmax": 15}))
    ):

        if make_norm:
            kwargs = {'norm': ForgivingNorm(**kwargs)}
        im_dict[j] = im = ax.imshow(zero_data, **kwargs)
        
        if early_autoscale_None:
            im.autoscale_None()
        a = f'pre: {im.get_clim()}'
        im.set_data(ramp_data)

        # this will happen eventually during the rendering, do it here
        # to capture the result
        im.autoscale_None()
        b = f'post: {im.get_clim()}'
        ax.set_title(f"{kwargs=}\n{a} {b}")

    fig.suptitle(f"{early_autoscale_None=} {make_norm=}")
    return fig, ax_arr, im_dict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants