-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add a colorbar() method to ScalarMappable? #25880
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
Comments
I like this idea. I'd much rather do |
Perhaps |
Not ideal to have a different name than That said, prior to v1.3 it was present but undocumented and returned a tuple of the colorbar and the colorbar axes, at that time it became an attribute and the |
I don't particularly object to mappables gaining a colorbar method. However, I don't understand the idea that colorbars have nothing to do with the figure - they are axes that are added to the figure, just like all other axes and they need to interact with the other axes on a figure is complex ways. While we create colorbars using a mappable, it's important to keep in mind that they are often organizationally attached and can steal space from whole groups of axes. |
I also think of the colorbar being conceptually attached to a figure (or axes). As a user, I usually don't want to bother with the mappable explicitly (similar to While in
for the OO interface we currently have to
I propose to go for
with the semantic: "if no mappable is given, check whether there is exactly one mappable in the figure. If so, use that, if not, raise "Multiple mappables found in the figure, please specify the mappable explicitly which should be used for the colorbar". " This follow the paradigm:
I find this more intuitive than
|
I'm not saying that it has nothing to do with the figure, but rather that the figure to which it gets attached is entirely independent of the figure object on which colorbar() is called, and rather entirely determined by either (the figure owning)
Note that with the proposal here, you could conveniently use method chaining:
I think such an API would be poor, for the following reason: In isolation, you cannot know whether a call to Similarly (it's really a variant on the same point), such an API would be quite brittle. Imagine that you write fig, axs = plt.subplot_mosaic([["image", "plot"]])
<some more code>
axs["image"].imshow(...)
fig.colorbar()
<some more code>
axs["plot"].plot(...) So far everything is good, but then you decide to add a third panel on the left with another image, which doesn't need a colorbar: fig, axs = plt.subplot_mosaic([["otherimage", "image", "plot"]])
<some more code>
axs["otherimage"].imshow(...)
<some more code>
axs["image"].imshow(...)
fig.colorbar()
<some more code>
axs["plot"].plot(...) Oops, all of a sudden the call to Still on the same idea, you can note that if "otherimage" was added on the right of "image", then the call to colorbar() would stay valid because at the moment of the colorbar() call there is still only one image in the figure, i.e. whether the call is valid or not also depends on the order of the subplots. |
I think that's sorta conflating two things though - semantically the colorbar is the legend for the colors in Besides the technical concerns of a fig, ax = plt.subplots()
im = ax.imshow(np.arange(0,100).reshape(10,10), cmap='RdBu')
fig.colorbar() but this didn't: fig, ax = plt.subplots()
im = ax.imshow(np.arange(0,100).reshape(10,10), cmap='RdBu')
sc = ax.scatter(np.linspace(0,9,5),np.linspace(9,0,5), c=np.linspace(0,1,5))
fig.colorbar() we'd end up almost immediately w/ feature requests for the latter, regardless of the warning message. (ETA: I'd put one in) |
Ok let's table parameter-less I'm strongly against It breaks with our dominant pattern that you add artists by calling methods on the parent artist that it gets added to (mostly Figure and Axes). From the artist hierarchy point of view, we don't add a colorbar to a mappable. Rather we add a colorbar to the figure and link it's content to a mappable. To me Additionally, for better or worse |
To be clear this is currently not the case for colorbars: as demonstrated by #25871 currently you can call a method on any figure (whether the mappable's (transitive) parent, or some unrelated figure), and the colorbar will get added to the mappable's parent figure, not to the one on which the method was called. We could add a check and error out if the [On a more personal PoV, I find that just annoying for the end user: often I write
then just work on ax/axs (I don't really need to carry around a fig variable which I mostly don't need), then if I need to write a colorbar I tend to just write I agree that the comparison with legend is a good one, but I guess this really means that the "clean" API would have been something like
as the colorbar really gets added to cax? |
The history here is that previously, The funny thing here is that adding a colorbar under the correct situations adjust the Axes layout of a A comprimise that I suspect no one will like is to add Each axes does have a (private) notion of "current image" which is what |
Isn't it simplest for mappable.colorbar to just be a short form for mappable.axes.figure.colorbar (probably not the correct attributes, but you get the idea)? It could then take all the arguments colorbar takes and default the ax argument to mappable.axes. That's a one liner and keeps the main method in figure. |
Yes, we should add that check. I consider the current behavior a bug.
I can relate to this. Directs access to the figure is rarely needed, and the extra
Possibly, but that's even more unwieldly than At the risk of dilluting the conversation: The idea behind my parameter-less
Yes, I fully agree that the drawbacks you mention in #25880 (comment) are there
I assume that is not too much of a problem in practice. (i)
We have Weighting all the above against always having to specify mappable, the parameter-less Further remark (Now going to really dangerous territory - please don't jump on this if you don't like it - this is only a quick idea and not at all thought through) I could even see an argument for a parameter-less
Disadvantages:
|
Hopefully it should be uncontroversial(!?!) that while the notions of current image/current axes/current figure is useful in pyplot and the pyplot API has its uses, these concepts should be kept as separated as possible from the OO core.
It can be that way or the other way round (figure.colorbar being a shorthand for mappable.colorbar, moving the implementation to ScalarMappable, note that you don't even need to get any attributes right here!), which is what I was thinking about, but obviously both ways are fine.
Certainly. That was a bit of a strawman. (But I guess @tacaswell's "proposal that no-one will like" is basically along the same lines.)
That's literally the same problem we always have with the stateful pyplot API here. Certainly "in practice" calling
If anything I'd rather have that, because it is much rarer to have two mappables on the same axes (two mappables on the same figure is extremely common for me, I'd say). But I still don't like it :) |
Yes. That's uncontroversial. - This was only a close and simple example that inserting code may affect later commands. We have more than enough other cases where that happens. |
See that's what I'm unhappy with since I think fig.legend grabs everything - and I'm maybe missing why fig.colorbar can't also grab everything and plot multiple colorbars? ETA: I keep thinking |
I think that's an interesting idea, but what would that call return? Returning a single colorbar in the case of a single mappable and a tuple/list of colorbars in the case of multiple mappables would result in a rather inelegant typing instability; always retuning a sequence of mappables would also not be so nice (cf. plot() which always returns a tuple of Line2Ds). |
That does only work when there is only one mappable per Axes (which is ok - we can and should error out if things get ambiguous). In that case, it would be equivalent to a looped call on a hypothetical parameter-less
At the risk of throwing more unfinshed API ideas around: For such a case, I have a rough vision, that we could replace the This would then allow
and of course variants like only setting colorbars at the most-right plots ( |
My interpretation of the proposal was rather to grab all mappables in the Axes and add a colorbar for each (and just have the colorbars next to one another). |
I see. That would be an extension/wider API of the parameter-less approach. My thought was keeping it simple and just not support multiple mappables in one Axes through one high-level function call. One can expand that, but that adds complexity (e.g. you cannot use |
I was thinking it could have a similar but simplified version of the legend API, so for example given these two mabbles: im = ax.imshow(np.arange(0,100).reshape(10,10), cmap='RdBu')
sc = ax.scatter(np.linspace(0,9,5),np.linspace(9,0,5), c=np.linspace(0,1,5)) these should be roughly equivalent calls and yes I'm also proposing that you can pass in a list of colorbars cause I think having to call the same function twice when you don't have to for legend seems weird unless you know the MPL internals.
map showing two meteorology variables, complex heatmap r package -> though both of these examples arrange colorbars more like legend entries
Yeah, I didn't know this and to Anthony's point about type instability and folks who do multiple colorbars wanting a legend type situation, if the answer for multiple colorbars isn't to have a legend like container object, but I also don't think that would work for the one horizontal/one vertical case either. I'm also not opposed to making this a new function (fig.colorbar_legend) instead. Going back to the original proposal of
I like |
Sorry to slightly hijack the thread here, but I think this may be a good opportunity to discuss whether
colorbar() could rather be a method on the ScalarMappable rather than on the Figure (we can always keep
figure.colorbar(...) available "forever", of course, for backcompat).
Indeed, in an call like
fig.colorbar(mappable)
orfig.colorbar(mappable, ax=...)
orfig.colorbar(mappable, cax=...)
,fig plays no role at all (*): the axes where the colorbar ends up is entirely determined by the mappable's axes (or by the ax
kwarg, or the cax kwarg.
(*) Actually I lied a bit: we currently check whether there's a layout engine on fig (but that actually seems more like a bug; we should really check that on mappable.axes.figure).
Originally posted by @anntzer in #25871 (comment)
The text was updated successfully, but these errors were encountered: