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

Fix rectangle and hatches for colorbar #23684

Merged
merged 1 commit into from Oct 6, 2022

Conversation

oscargus
Copy link
Contributor

@oscargus oscargus commented Aug 20, 2022

PR Summary

Closes #23456.

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

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

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This fix seems correct to fix the shape bug. However, I'll block because It seems better to just leave hatching completely broken, for the horizontal case at least, rather than fix it for a 3.6 with an off-by-one bug? I appreciate the off-by-one bug is in the semi-working vertical colorbar as well, and agree that they both need to be fixed. If anything, I'd remove the hatching completely from the colorbar for 3.6 until there is a proper fix.

@jklymak
Copy link
Member

jklymak commented Aug 20, 2022

As per comment in the original issue if this fixes cbars without extends then I withdraw my objection. Obviously we should fix the extends but this is an improvement. I'd add a test with no extends
For now.

@jklymak jklymak dismissed their stale review August 20, 2022 19:36

Problem just with extends. This fixes non extend cbars properly

@oscargus
Copy link
Contributor Author

I've fixed the extends as well now.

def test_colorbar_contourf_extend_patches():
params = [
('both', 5, ['\\', '//']),
('min', 7, ['+']),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may seem stupid, but this did actually break at one stage as hatches wasn't extended enough.

cs = ax.contourf(x, y, z, levels, hatches=hatches,
cmap=cmap, extend=extend)
fig.colorbar(cs, ax=ax, orientation=orientation, fraction=0.4,
extendfrac=0.2, aspect=5)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Oversized" colorbar so that hatches are visible.

@oscargus oscargus changed the title Fix rectangle for horizontal colorbar with hatches Fix rectangle and hatches for colorbar Aug 20, 2022
@oscargus oscargus added this to the v3.7.0 milestone Aug 30, 2022
@jklymak jklymak modified the milestones: v3.7.0, v3.6.1 Sep 8, 2022
@jklymak
Copy link
Member

jklymak commented Sep 8, 2022

Moved to 3.6.1 since this is a bug fix, even if its an old bug?

lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_colorbar.py Outdated Show resolved Hide resolved
@oscargus
Copy link
Contributor Author

I'll try to update this over the weekend. Had forgotten about the comments...

lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
lib/matplotlib/colorbar.py Outdated Show resolved Hide resolved
@QuLogic QuLogic merged commit 60d3d29 into matplotlib:main Oct 6, 2022
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 6, 2022
QuLogic added a commit that referenced this pull request Oct 6, 2022
…684-on-v3.6.x

Backport PR #23684 on branch v3.6.x (Fix rectangle and hatches for colorbar)
@oscargus oscargus deleted the colorbarhatchrectangle branch October 6, 2022 10:40
@ksunden ksunden mentioned this pull request Feb 20, 2023
6 tasks
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.

[Bug]: Horizontal colorbars drawn incorrectly with hatches
3 participants