Colorbar inherit from Axes #20350
Colorbar inherit from Axes #20350
Conversation
This is for clarity that the colorbar is now an Axes itself.
9ff6baa to
895776b
Compare
| self.ax = ax | ||
| ax.set(navigate=False) | ||
| super().__init__(ax, userax=userax) | ||
| self.ax = self |
There was a problem hiding this comment.
I'd suggest making this a property instead: it would make room to add a warning when this "attribute" is deprecated.
There was a problem hiding this comment.
We can always transparently turn an attribute into a property later if we want to add a warning. That's one of the great features of properties.
There was a problem hiding this comment.
That's right. I guess you could see this as premature :)
|
Now that 3.5 is in, its probably reasonable to reconsider this. Can we succinctly define what a Colorbar is versus an Axes? I think I am starting to agree that saying a colorbar is a special axes may be helpful from an API point of view... |
|
I'm skeptical on this. Generally, "favor composition over inheritance". I think inheritance here would be the lazy way to make some desired axes API available on the colorbar. However, that comes at the cost of exposing a lot of unneeded other API, and it may couple us more to the Axes behavior than we want. I suggest to go with composition and add relevant properties and methods to the colorbar that delegate to the underlying Axes. |
|
Well we already do composition so we are covered there. The issue with wrapping the lower level API is that they drift apart because we change the axes or axis API and it doesn't get changed on colorbar. Similarly, colorbar ends up with a mishmash of Axes and Axis wrappers, which is already a mess as Axes already wraps quite a few Axis methods. So for locators and ticks we have three ways on colorbar - directly changing them, changing them on the Axes, and changing them on the Axis. If we were to continue wrap, then I'd argue we should consider not giving access to the Axes. |
I expect our API to be rather stable 😎. But if that's of real concern we could add tests for that.
Agreed. We should have a direct and simple interface on the colorbar itself. Everything else can cause trouble for developers and/or users in the long run by exposing more detail than necessary. |
PR Summary
A Colorbar currently stores the Axes that is used for drawing in the cbar.ax attribute, which many of the methods are then called internally like self.ax.pcolormesh(). My proposal is to move the Axes up a level and have Colorbar inherit from the new CbarAxes class (to become an Axes itself) and turn the previous call into self.pcolormesh() dropping ax attribute.
Benefits: This would help with interactivity as proposed in #19515, by being able to identify if an axes is a Colorbar or not and then controlling properties based on that. It would also help with #20330, by giving the CbarAxes the ability to remove colorbar callbacks.
Drawbacks: This adds a lot of easy-to-use methods onto a Colorbar now that may not be desired to directly expose to users. i.e. they could do cbar.plot() now, whereas before cbar.ax.plot() hides those capabilities a little bit more.
closes #20341
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).