Skip to content
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

ENH: add colorbar info to gridspec cbar #20414

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jun 11, 2021

PR Summary

Non-gridspec colorbar axes have a _colorbar_info appended to them, but the gridspec and user-supplied ones do not. This PR adds the attribute to all colorbar axes, and is set to 'user' if the axes is provided by the user.

This is helpful in cases where we want to tell if an axes is a colorbar or not, though all colorbar axes should also be of type ColorbarAxes, so maybe not strictly needed...

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@jklymak jklymak added this to the v3.5.0 milestone Jun 11, 2021
parents=[parent],
shrink=shrink,
anchor=anchor,
panchor=panchor,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to be read anywhere? Is that a bug elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No the point is just to be consistent with the non-gridspec axes. In particular we can now check for the attribute. However, after I put this in I realized that we can check if an axes is a colorbar more simply just by whether it is a ColorbarAxes, so maybe this isn't really needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I only meant panchor, but if you think there's a better way to do this entirely..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see. I included panchor just as a precaution - we may want to handle it better in the future.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While dynamically adding attributes as needed is possible in python, people have found that it's hard to use because you never know which attributes are available (and when) on an object. And getattr/ hasattr are much harder to use than self._colrobar_info is None.

Therefore +1 for making this a regular field of a ColorbarAxes.

@QuLogic QuLogic merged commit 6014058 into matplotlib:master Jun 16, 2021
@jklymak jklymak deleted the enh-colorbar_info branch June 16, 2021 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants