Correctly pass location when constructing ImageGrid colorbar.#25884
Merged
timhoffm merged 1 commit intomatplotlib:mainfrom May 14, 2023
Merged
Correctly pass location when constructing ImageGrid colorbar.#25884timhoffm merged 1 commit intomatplotlib:mainfrom
timhoffm merged 1 commit intomatplotlib:mainfrom
Conversation
Previously only the orientation would be passed, so information on whether the colorbar is on the top or the right would lost, resulting in ticks, ticklabels, and label being on the wrong side of the colorbar. Fix that. The baseline image changes exactly test that (the colorbar ticks move to the other side). While at it, also simplify the plotted data in test_imagegrid_cbar_mode, which decreases more than 2x the size of the baseline image (86K -> 34K).
6 tasks
oscargus
approved these changes
May 14, 2023
timhoffm
approved these changes
May 14, 2023
| @@ -22,23 +22,14 @@ def __init__(self, *args, orientation, **kwargs): | |||
| self.orientation = orientation | |||
| super().__init__(*args, **kwargs) | |||
Member
There was a problem hiding this comment.
Semi-OT, but got to this because you deleted cla(): This super().__init__ should not do anything, right? - Likely a leftover from the dedicated axes_grid colorbar classes?
Contributor
Author
There was a problem hiding this comment.
It does (should?), as CbarAxesBase is a mixin and will thus end up in the base classes of the actual colorbar axes class used by ImageGrid, and thus we need to forward arguments via super() (after extracting out orientation).
Member
There was a problem hiding this comment.
Indeed. Maybe that should be mentioned somewhere in the class? without that context, it's looks like object is the only parent. Either way, the PR is correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously only the orientation would be passed, so information on whether the colorbar is on the top or the right would lost, resulting in ticks, ticklabels, and label being on the wrong side of the colorbar. Fix that.
The baseline image changes exactly test that (the colorbar ticks move to the other side). While at it, also simplify the plotted data in test_imagegrid_cbar_mode, which decreases more than 2x the size of the baseline image (86K -> 34K).
Closes #23595.
I think this was really broken by https://github.com/matplotlib/matplotlib/pull/20501/files#diff-ba79d31808d1f55bdf8bf2896366c85122eb9488e140c5a4de060f2c4e1fa8f6L42.
PR summary
PR checklist