Skip to content

Passing clim as keyword argument to pcolormesh does not change limits. #10346

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
heathhenley opened this issue Jan 30, 2018 · 9 comments · Fixed by #10354
Closed

Passing clim as keyword argument to pcolormesh does not change limits. #10346

heathhenley opened this issue Jan 30, 2018 · 9 comments · Fixed by #10354

Comments

@heathhenley
Copy link
Contributor

heathhenley commented Jan 30, 2018

Bug report

Bug summary

Not sure if this is really a bug or just a user issue. I expected that passing a clim tuple to pcolormesh with no vmin or vmax specified would update the color scale limits. I found that I had to use vmin and vmax instead. Maybe I am misunderstanding and vmin and vmax should always be used. However, it looks like the kwargs are passed into the QuadMesh constructor and then clim is overwritten with vmin and vmax, even in they are None and clim is given. (https://github.com/heath730/matplotlib/blob/master/lib/matplotlib/axes/_axes.py#L5629).

Code for reproduction

import matplotlib.pyplot as plt
import numpy as np


plot_me = np.random.rand(5, 5)

plt.subplot(211)
plt.title("Expected")
plt.pcolormesh(plot_me, vmin=0, vmax=0.5)
plt.colorbar()

plt.subplot(212)
plt.title("Result")
plt.pcolormesh(plot_me, clim=(0, 0.5))
plt.colorbar()

plt.show()

figure_1

Matplotlib version

  • Operating system: win7
  • Matplotlib version: 2.1.2
  • Matplotlib backend (print(matplotlib.get_backend())): TkAgg
  • Python version: 3.5.1
  • Jupyter version (if applicable):
  • Other libraries:

Would something like:

  (vmin, vmax) = kwargs.pop('clim', (None, None))

Before the vmin and vmax pops on this line be a reasonable update?

@jklymak
Copy link
Member

jklymak commented Jan 30, 2018

Do the docs say that you can pass clim? I couldn't find anywhere that they say you can. There is a pyplot.clim but thats a matlab-ism...

I think having a clim argument and vmin/vmax is asking for trouble...

@heathhenley
Copy link
Contributor Author

@jklymak They don't list clim as an arg for pcolormesh, but they say the properties of the QuadMesh object that pcolormesh creates can be controlled using kwargs (including clim). However, if you do pass clim it will get overwritten with vmin/vmax. I guess just using vmin/vmax is probably the simplest way to go though, I did come from a matlab background originally so I guess that's why I was trying to use clim instead of vmin/vmax in the first place.

The docs I'm referring to are: https://matplotlib.org/api/_as_gen/matplotlib.axes.Axes.pcolormesh.html

@jklymak
Copy link
Member

jklymak commented Jan 31, 2018

Yep you are right. Thats actually pretty annoying. Not sure what should change. It should be possible to not override clim if vmin and vmax are not provided. But ideally both forms would not exist. I'm not sure what the history is here.

@efiring
Copy link
Member

efiring commented Jan 31, 2018

This is long-ago cleverness coming back to bite us. The clim kwarg is being inherited based on the ScalarMappable.set_clim method, via the call to Artist.update in the Collection.__init__. That's why we have all these horrendous tables clogging the documentation: mostly irrelevant "properties", also advertised as "kwargs" and inherited via this circuitous process.

@heathhenley
Copy link
Contributor Author

Do you think a note in the docstring that the clim property of the returned QuadMesh will be replaced by vmin/vmax would be appropriate or is it better to leave as is? If I understand your explanation correctly, pcolormesh is not really meant to support clim, but that table is being pulled into the docs because of the class hierarchy.

@efiring
Copy link
Member

efiring commented Jan 31, 2018

The docstring is autogenerated from pieces pulled from various places, and used in many places. Maybe it would make sense to add a note to the set_clim docstring to the effect that it should not be used when vmin, vmax are available.

I suspect this clash between clim and vmin, vmax is widespread, and there are probably many other such inherited properties that it would be better not to expose as kwargs. I haven't thought it through, though.

@anntzer
Copy link
Contributor

anntzer commented Jan 31, 2018

fwiw I would just pass everything to QuadMesh and teach ScalarMappable how to handle them.

diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
index df2ccd55e..77b9a8089 100644
--- a/lib/matplotlib/axes/_axes.py
+++ b/lib/matplotlib/axes/_axes.py
@@ -5866,11 +5866,6 @@ linewidth=2, markersize=12)
         if not self._hold:
             self.cla()
 
-        alpha = kwargs.pop('alpha', None)
-        norm = kwargs.pop('norm', None)
-        cmap = kwargs.pop('cmap', None)
-        vmin = kwargs.pop('vmin', None)
-        vmax = kwargs.pop('vmax', None)
         shading = kwargs.pop('shading', 'flat').lower()
         antialiased = kwargs.pop('antialiased', False)
         kwargs.setdefault('edgecolors', 'None')
@@ -5892,14 +5887,7 @@ linewidth=2, markersize=12)
         collection = mcoll.QuadMesh(Nx - 1, Ny - 1, coords,
                                     antialiased=antialiased, shading=shading,
                                     **kwargs)
-        collection.set_alpha(alpha)
         collection.set_array(C)
-        if norm is not None and not isinstance(norm, mcolors.Normalize):
-            raise ValueError(
-                "'norm' must be an instance of 'mcolors.Normalize'")
-        collection.set_cmap(cmap)
-        collection.set_norm(norm)
-        collection.set_clim(vmin, vmax)
         collection.autoscale_None()
 
         self.grid(False)
diff --git a/lib/matplotlib/cm.py b/lib/matplotlib/cm.py
index ecd1179f1..c6cc87472 100644
--- a/lib/matplotlib/cm.py
+++ b/lib/matplotlib/cm.py
@@ -318,6 +318,12 @@ class ScalarMappable(object):
             self.norm.vmax = vmax
         self.changed()
 
+    def set_vmin(self, vmin):
+        self.set_clim(vmin=vmin)
+
+    def set_vmax(self, vmax):
+        self.set_clim(vmax=vmax)
+
     def set_cmap(self, cmap):
         """
         set the colormap for luminance data

or, if we decide that's not the route we want to take, we should just deprecate clim, instead of keeping it half-assed...

Note that the above solution will apply kwargs in the order they are given, so 1) that only works on Py3.6+ which keeps kwargs order (by mpl3 is coming :)) and 2) requires the user to pass the norm before the clims, which does not seem unreasonable...

We also have a precedent of just forcing the order of the kwargs to something logically consistent in https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/text.py#L178.

@jklymak
Copy link
Member

jklymak commented Jan 31, 2018

You can do that. But you need to decide what takes precedence - vmin/vmax or clim. That’s pretty bad API if there are two ways to do the same thing with arbitrary precedence. I’d prefer to deprecate clim.

Reopening because the documentation change is fine but not really the root of the problem.

@jklymak jklymak reopened this Jan 31, 2018
@anntzer
Copy link
Contributor

anntzer commented Nov 2, 2021

I think this was effectively fixed by #21146. Feel free to reopen if not.

@anntzer anntzer closed this as completed Nov 2, 2021
@QuLogic QuLogic added this to the v3.5.0 milestone Nov 2, 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 a pull request may close this issue.

5 participants