| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
ping @efiring. Would this address #22087 or do we need to do more major surgery? @greglucas may also have some opinions. Thanks! |
Sorry, something went wrong.
|
The method you have moves this up to the colorbar level, so it wouldn't help the issue where a user is explicitly calling the method on the colorbar cbar.ax.set_aspect(). I guess I still see a disconnect between the Colorbar methods and the axes method in this case, so somehow those need to be brought in-sync so that a user could use either method and get the same results, otherwise I think we are going to run into issues with people using one method versus another. I think the underlying issue is that _colorbar_info is stale, and so that needs to be updated somewhere if someone calls cbar.ax.set_aspect(). This patch appears to work, but maybe isn't all that elegant either... diff --git a/lib/matplotlib/colorbar.py b/lib/matplotlib/colorbar.py
index b67b108891..58c2928ea1 100644
--- a/lib/matplotlib/colorbar.py
+++ b/lib/matplotlib/colorbar.py
@@ -254,6 +254,14 @@ class _ColorbarAxesLocator:
aspect = ax._colorbar_info['aspect']
else:
aspect = False
+
+ if not isinstance(self._cbar.ax.get_aspect(), str):
+ # It was explicitly set by the user, so we need to
+ # update the stored value in colorbar_info
+ aspect = self._cbar.ax.get_aspect()
+ ax._colorbar_info['aspect'] = aspect
+ self._cbar.ax.set_aspect("auto")
+
# now shrink and/or offset to take into account the
# extend tri/rectangles.
if self._cbar.orientation == 'vertical':Perhaps creating a new set_aspect() method that you can replace the standard set_aspect with and updates the colorbar info as well? def set_aspect(self, aspect, **kwargs):
self._colorbar_info["aspect"] = aspect
super().set_aspect(aspect, **kwargs)
self.ax.set_aspect = set_aspect |
Sorry, something went wrong.
|
Hmmm... and what about the idea of having make_axes\_gridspec return a wrapped axes method (ColorbarAxes). Note that would be different than the old proposal which wrapped all colorbars that way. This would only wrap the ones we make. That seems more elegant to me, and would fix @efiring 's problem. |
Sorry, something went wrong.
|
I'm still trying to figure out exactly what the situations are that need to be covered, keeping in mind
|
Sorry, something went wrong.
|
Sorry, dropped this. The fundamental API problem is that we have an axes that helps make the colorbar, either supplied by the user, or we make one. Complicating things, we allow the user to access that axes, and some of the things we want them to twiddle we only give them twiddling access via cb.ax. However, the way we make colorbars, in particularly with extends, calling cb.ax.set_aspect is not really meaningful, as we alter the axes to be shorter to accommodate the extends and maintain the same overall aspect including the extends. To me, the best API would be that the user set the aspect on the colorbar, not the axes, which is now an implementation detail. I don't know that there is a way to prevent cb.ax.set_aspect, but I'm not terribly concerned if it doesn't work if we provide the user with a top-level API for the colorbar. I'd argue we should do that for most other things we are currently doing by twiddling cb.ax. |
Sorry, something went wrong.
|
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
Replaces #20588
Closes #22087 (?)
PR Checklist
Tests and Styling
Documentation