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

I'm -0.5 on this change. The implicit API is a bit goofy anyway, but there are plenty of reasons to have the colorbar steal from the current axes rather than the mappable's axes. I guess I think its more unexpected for

plt.subplots(1, 2, 1)
pcm = plt.pcolormesh(X)
plt.subplots(1, 2, 2)
plt.plot(y)
plt.colorbar(pcm) 

to put the colorbar next to the first subplot than next to the one I specified last.

For the change to colorbar() to work we need ScalarMappables to have an .axes, so ContourSets need to grow such an attribute (... or we need to special-case ContourSets in the colorbar() code, which seems worse). We don't have to deprecate .ax, we can leave both of them around, but it seems just as good to make things consistent.
I can undeprecate ContourSet.ax if that's the main issue...

While .ax and .axes may be nearly as common in raw counts in the codebase, I'd guess .axes is much more common in actual use because that's what Artists have, and Artists also share a lot of code in the base class, whereas .ax is on a couple of custom non-Artist things (e.g. ContourSets) with each their own codepaths.

As for

plt.subplots(1, 2, 1)
pcm = plt.pcolormesh(X)
plt.subplots(1, 2, 2)
plt.plot(y)
plt.colorbar(pcm) 

if you make your plots vertically (211 and 212 instead of 121 and 122), I think the new behavior is better...

TBH I don't really care about plt.colorbar(), but at least for fig.colorbar() I'd rather not invoke the notion of "current axes".

TBH I don't really care about plt.colorbar(), but at least for fig.colorbar() I'd rather not invoke the notion of "current axes".

OK, I agree with that.

I don't object to axes being universal versus ax, but I wasn't aware we had decided that. And as noted above an effort should be made to do it everywhere. Understood about needing the attribute.

We haven't decided it, it's just needed for this specific PR to work (and again I can undeprecate .ax for this specific PR if you prefer). Making the change everywhere is absolutely not the point of this PR.

If you insist we can also make plt.colorbar() use the current axes but fig.colorbar() the mappable's axes, but that seems worse...

👍, but we should decide on the deprecation...

For the change to colorbar() to work we need ScalarMappables to have an .axes, so ContourSets need to grow such an attribute.

Thanks for the explanation. Having ax and axes is not good either. Even though it's not the target of this PR, can we decide on a global change from ax to axes in attribute names? That would allow to keep the deprecation in here and add the remaining ones in another PR.

I'd be happy with the change but I'm sure someone will complain about useless code churn, etc :)

@anntzer To move this forward, you could split out the ax -> axes deprecation and move that to #16058. It's better to discuss it in that context.

OK, I moved the relevant changes to #16058 and will rebase this one on top of it once it goes in.

rebased

Actually, looking at this again, do we want to have the same behavior for pyplot.colorbar too? Obviously pyplot.colorbar gets to know what the current image (gci()) is, but it likely makes sense to always use gci()'s axes, not the current axes? (for the same reason why this makes sense for the OO API)

Added the warning for pyplot.colorbar as a separate commit.
Edit: This actually revealed a bug in the original implementation: we should not warn if cax is set, as in that case we're not going to steal from ax anyways. Fixed.