ENH: add colorbar method to axes #12333

jklymak · 2018-09-28T23:21:19Z

PR Summary

It has always struck me as strange that if we want to add a colorbar to an axes we have to call if from figure. This change just lets you do ax.colorbar(mappable).

Note that ax.colorbar(mappable, ax=axother) will just ignore the ax argument.

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

ImportanceOfBeingErnest · 2018-09-28T23:27:38Z

lib/matplotlib/axes/_axes.py

+        %(colorbar_doc)s
+        """
+        self.figure.colorbar(self, mappable, cax=None, ax=self,
+                             use_gridspec=True, **kw)


I think it needs to be

self.figure.colorbar(mappable, cax=cax, ax=self, use_gridspec=use_gridspec, **kw)

and maybe you want to pop ax out of kw already within this method? Else (I think) it should raise an error if ax is given twice to as argument.

Yep! Sorry, should have marked work in progress, I havent' tested it , and it needs tests in test_axes

ImportanceOfBeingErnest · 2018-09-29T00:23:51Z

The true advantage of this would be if mappable was optional as well, i.e. if calling ax.colorbar() on an ax which contains a (single) image or collection would - similar to plt.colorbar() - just work out of the box.

jklymak · 2018-09-29T01:30:27Z

I think we are trying to avoid implicit APIs like that.

jklymak · 2018-09-29T04:02:26Z

... though I guess fig.colorbar implicitly uses gca if ax is not supplied.

ImportanceOfBeingErnest · 2018-09-29T13:11:59Z

I would hope fig.colorbar(mappable) uses mappable.axes. Anyways, I would see this similar to ax.legend(). If you don't specify handles and labels it will automatically try to determine the handles and labels to show from the children of ax, via ax.get_legend_handles_labels(). If there are no artists with a label present it will throw a warning. And I think plt.colorbar() does that already. The difference to legend is that if you have more than one ScalarMappable in the axes, ax.colorbar() would need to warn as well.

Thinking that even one step further, fig.legend() also does not require any arguments. So one could also make the mappable optional for fig.colorbar(ax=ax) as well.

anntzer · 2018-09-29T15:09:08Z

lib/matplotlib/axes/_axes.py

+        """
+        ax = kw.pop('ax', None)
+        if ax is not None:
+            warnings.warn('Supplied ax argument ignored')


I would just make this an error, not much point of being tolerant in a new API...

I'd say it pretty much depends on how the documentation is written. If it says "allowed kwargs are all matplotlib.figure.Figure.colorbar arguments", then it should warn. If it explicitely says "allowed kwargs are all matplotlib.figure.Figure.colorbar arguments, except ax", it can error out.

👍 to raising. In someplaces we have to tolerate ignoring input because it is old API, but we should not add ignored kwargs in new API.

WeatherGod · 2018-09-29T15:14:31Z

I am -0 on this. Yes, I have been bitten before when a call to `ax.colorbar()` raises an attribute exception, but then I remember that a colorbar is not owned by an axes, it is owned by the figure. Having `ax.colorbar()` work would imply that the colorbar is owned by the axes, which is not the case.

…

On Sat, Sep 29, 2018 at 11:09 AM Antony Lee ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/matplotlib/axes/_axes.py <#12333 (comment)> : > @@ -418,6 +418,21 @@ def legend(self, *args, **kwargs): def _remove_legend(self, legend): self.legend_ = None + @docstring.dedent_interpd + def colorbar(self, mappable, cax=None, use_gridspec=True, **kw): + """ + Create a colorbar for a ScalarMappable instance, *mappable*, with the + axes as the parent. + + Documentation for the pyplot thin wrapper: + %(colorbar_doc)s + """ + ax = kw.pop('ax', None) + if ax is not None: + warnings.warn('Supplied ax argument ignored') I would just make this an error, not much point of being tolerant in a new API... — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#12333 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-MlKNwDxN0ssUnokTlkqlqgvRGDIks5uf40WgaJpZM4XAMhk> .

ImportanceOfBeingErnest · 2018-09-29T16:06:52Z

a colorbar is not owned by an axes, it is owned by the figure.

From a user's perspective, a colorbar most often "belongs" to an axes. Also it strongly interferes with it in the default case where it steals space from the axes. (In which case one could argue inversely as to why a figure level function is allowed to cut axes.)

jklymak · 2018-09-29T17:03:14Z

I would hope fig.colorbar(mappable) uses mappable.axes.

matplotlib/lib/matplotlib/figure.py

Lines 2128 to 2129 in 85c9742

    
           if ax is None: 
        
               ax = self.gca()

Thats the problem w/ implicit APIs, the API sometimes doesn't do what you expect 😉 Not 100% against making this more implicit, but axes can have more than one mappable, so I find this a bit dangerous.

@WeatherGod I can see that viewpoint. But given that 90%+ of colorbars are attached to a single axes, I think the shortcut proposed here is reasonable API. The user never really sees our organization so they don't really need to know about it. OTOH, the drawback is that it hides the more flexible .Figure.colorbar.

WRT the docs, they are a bit of a mess, so that can/should be cleaned up. But I'm not willing to do it yet because how much needs to be done depends on whether this new API is deemed acceptable. If folks are OK w/ the new API, I can also refactor the docs a bit so plt.colorbar, figure.colorbar, and axes.colorbar have more specific signature descriptions, and then the kwargs are shared.

tacaswell · 2018-09-29T20:42:29Z

I am also -0.5 to -0 on this for the same reasons @WeatherGod said. Managing child axes is the business of the figure. I am also not sure that the cax kwarg makes a whole lot of sense an the Axes object

I am nominally 👍 on switching Figure.colorbar to steal from the axes of the mappable object rather than the figure's current axes.

WeatherGod · 2018-09-29T21:33:45Z

Another argument against this... how should `cla()` behave for an axes that had a colorbar created via `ax.colorbar()`? It might be confusing that the colorbar doesn't go away in that situation.

…

On Sat, Sep 29, 2018 at 4:42 PM Thomas A Caswell ***@***.***> wrote: I am also -0.5 to -0 on this for the same reasons @WeatherGod <https://github.com/WeatherGod> said. Managing child axes is the business of the figure. I am also not sure that the cax kwarg makes a whole lot of sense an the Axes object I am nominally 👍 on switching Figure.colorbar to steal from the axes of the mappable object rather than the figure's current axes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12333 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-Ob_T57pszYnyESiDb-hWzQ4rIJ3ks5uf9s3gaJpZM4XAMhk> .

jklymak · 2018-09-29T21:41:35Z

I just think ax.colorbar is more discoverable and what I instinctively think of doing first before I remember I can’t do that. And I think I’m as familiar with how colorbars are implemented as most users. But I don’t feel at all strongly about this if it’s not a widely shared view and just satisfying a personal mental tick.

ImportanceOfBeingErnest · 2018-09-30T00:31:31Z

It is a very matplotlib specific implementation detail that a legend is a child artist of an axes, while a colorbar needs its own axes to reside in. From the user's "I want to show an explanation of the contents of my plot"-perspective this distinction is unnecessary and confusing. At some point the user will need to understand the difference (e.g. in the case of clearing the axes mentionned above), but with the proposed ax.colorbar the required level of understanding is lower for the average task of creating a colorbar next to an axes. Or the other way around: a user can do more stuff at the same level of understanding before he needs to acquire new knowledge about further implementation details.

Also relevant: Recently a new ax.inset_axes got introduced. This creates an axes, starting from an axes. With the argument that only figure methods should create axes, this would have needed to be fig.inset_axes(ax=ax).

jklymak · 2018-09-30T02:29:22Z

Also relevant: Recently a new ax.inset_axes got introduced. This creates an axes, starting from an axes. With the argument that only figure methods should create axes, this would have needed to be fig.inset_axes(ax=ax).

Well, just to clarify, the inset axes is actually a child artist of the axes object because we now allow child axes. Not that I am suggesting we should refactor colorbar so that single-axes colorbars are child axes...

anntzer · 2018-09-30T10:19:08Z

My initial reaction was that I like the proposed API, but thinking about it again after reading the other comments I guess the most logical API would be to have ScalarMappable.colorbar(ax=self.axes, *, cax, **kwargs)?

jklymak · 2018-10-03T23:27:22Z

Any other opinions on this? So far the consensus seems mildly opposed, with one vote for scalarmappable.colorbar 😉 Happy to scrub it - its maybe more discoverable, but on the other hand it makes it harder to discover the more flexible figure.colorbar

jklymak · 2018-10-05T15:27:51Z

Closing due to lack of love for the idea 😉

jklymak · 2018-10-25T23:47:11Z

See https://stackoverflow.com/questions/52987661/use-of-colorbar-matplotlib-associated-to-a-axes for someone who wanted this feature 😉

jklymak added this to the v3.1 milestone Sep 28, 2018

jklymak force-pushed the enh-add-colorbar-axes branch from 608cc7f to 6e896a7 Compare September 28, 2018 23:22

ImportanceOfBeingErnest reviewed Sep 28, 2018

View reviewed changes

jklymak force-pushed the enh-add-colorbar-axes branch from 6e896a7 to e7cb45e Compare September 28, 2018 23:31

jklymak added the status: work in progress label Sep 28, 2018

jklymak force-pushed the enh-add-colorbar-axes branch from e7cb45e to 081a4f5 Compare September 29, 2018 03:57

ENH: add colorbar method to axes

0084101

jklymak force-pushed the enh-add-colorbar-axes branch from 081a4f5 to 0084101 Compare September 29, 2018 04:01

jklymak removed the status: work in progress label Sep 29, 2018

anntzer reviewed Sep 29, 2018

View reviewed changes

jklymak closed this Oct 5, 2018

jklymak deleted the enh-add-colorbar-axes branch October 5, 2018 15:27

anntzer mentioned this pull request Oct 8, 2018

Warn in colorbar() when mappable.axes != figure.gca(). #12443

Merged

6 tasks

anntzer mentioned this pull request Aug 25, 2022

[Bug]: colorbar with unattached mappables can't steal space #23709

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: add colorbar method to axes #12333

ENH: add colorbar method to axes #12333

jklymak commented Sep 28, 2018 •

edited

Loading

ImportanceOfBeingErnest Sep 28, 2018

jklymak Sep 28, 2018

ImportanceOfBeingErnest commented Sep 29, 2018

jklymak commented Sep 29, 2018

jklymak commented Sep 29, 2018

ImportanceOfBeingErnest commented Sep 29, 2018

anntzer Sep 29, 2018

ImportanceOfBeingErnest Sep 29, 2018

tacaswell Sep 29, 2018

WeatherGod commented Sep 29, 2018 via email

ImportanceOfBeingErnest commented Sep 29, 2018

jklymak commented Sep 29, 2018

tacaswell commented Sep 29, 2018

WeatherGod commented Sep 29, 2018 via email

jklymak commented Sep 29, 2018 •

edited

Loading

ImportanceOfBeingErnest commented Sep 30, 2018

jklymak commented Sep 30, 2018

anntzer commented Sep 30, 2018 •

edited

Loading

jklymak commented Oct 3, 2018

jklymak commented Oct 5, 2018

jklymak commented Oct 25, 2018

ENH: add colorbar method to axes #12333

ENH: add colorbar method to axes #12333

Conversation

jklymak commented Sep 28, 2018 • edited Loading

PR Summary

PR Checklist

ImportanceOfBeingErnest Sep 28, 2018

Choose a reason for hiding this comment

jklymak Sep 28, 2018

Choose a reason for hiding this comment

ImportanceOfBeingErnest commented Sep 29, 2018

jklymak commented Sep 29, 2018

jklymak commented Sep 29, 2018

ImportanceOfBeingErnest commented Sep 29, 2018

anntzer Sep 29, 2018

Choose a reason for hiding this comment

ImportanceOfBeingErnest Sep 29, 2018

Choose a reason for hiding this comment

tacaswell Sep 29, 2018

Choose a reason for hiding this comment

WeatherGod commented Sep 29, 2018 via email

ImportanceOfBeingErnest commented Sep 29, 2018

jklymak commented Sep 29, 2018

tacaswell commented Sep 29, 2018

WeatherGod commented Sep 29, 2018 via email

jklymak commented Sep 29, 2018 • edited Loading

ImportanceOfBeingErnest commented Sep 30, 2018

jklymak commented Sep 30, 2018

anntzer commented Sep 30, 2018 • edited Loading

jklymak commented Oct 3, 2018

jklymak commented Oct 5, 2018

jklymak commented Oct 25, 2018

jklymak commented Sep 28, 2018 •

edited

Loading

jklymak commented Sep 29, 2018 •

edited

Loading

anntzer commented Sep 30, 2018 •

edited

Loading