Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

ENH: add colorbar.set_aspect#22103

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
jklymak wants to merge2 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromjklymak:enh-colorbar-set-aspect

Conversation

jklymak
Copy link
Member

PR Summary

Replaces#20588

Closes#22087 (?)

PR Checklist

Tests and Styling

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

@jklymak
Copy link
MemberAuthor

ping@efiring. Would this address#22087 or do we need to do more major surgery?@greglucas may also have some opinions. Thanks!

@greglucas
Copy link
Contributor

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 colorbarcbar.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 callscbar.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.pyindex 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 newset_aspect() method that you can replace the standard set_aspect with and updates the colorbar info as well?

defset_aspect(self,aspect,**kwargs):self._colorbar_info["aspect"]=aspectsuper().set_aspect(aspect,**kwargs)self.ax.set_aspect=set_aspect

@jklymak
Copy link
MemberAuthor

Hmmm... and what about the idea of havingmake_axes\_gridspec return a wrapped axes method (ColorbarAxes). Note that would be different than the old proposal which wrappedall colorbars that way. This would only wrap the oneswe make. That seems more elegant to me, and would fix@efiring 's problem.

greglucas reacted with thumbs up emoji

@efiring
Copy link
Member

I'm still trying to figure out exactly what the situations are that need to be covered, keeping in mind

  • Can a clean solution work with old code and new, or with only minimal changes to old code?
  • What is the future API we really want?
    I think that one of the problems is handling the situation where the cax is not created by stealing space from an existing Axes, but is created outside and passed in to thecolorbar. Wouldn't the ideal API be one that minimizes differences between the "we made it by stealing" case and the "made elsewhere and was passed in" case with respect to how kwargs and methods work and what can be done? If I understand correctly,@greglucas is heading in that direction.

@jklymak
Copy link
MemberAuthor

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 weonly 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 aspectincluding 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 preventcb.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 twiddlingcb.ax.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: aspect ratio control of colorbar Axes fails.
3 participants
@jklymak@greglucas@efiring

[8]ページ先頭

©2009-2025 Movatter.jp