FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix rectangle and hatches for colorbar by oscargus · Pull Request #23684 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Fix rectangle and hatches for colorbar - #23684

Merged
QuLogic merged 1 commit into
matplotlib:mainfrom
oscargus:colorbarhatchrectangle
Oct 6, 2022
ConversationCommits (1)ChecksFiles changed
Merged

Fix rectangle and hatches for colorbar#23684
QuLogic merged 1 commit into
matplotlib:mainfrom
oscargus:colorbarhatchrectangle

Conversation

oscargus commented Aug 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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).

jklymak previously requested changes Aug 20, 2022
View reviewed changes

jklymak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Aug 20, 2022

Copy link
Copy Markdown
Member

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 dismissed their stale review August 20, 2022 19:36

Problem just with extends. This fixes non extend cbars properly

oscargus force-pushed the colorbarhatchrectangle branch from 5aaf547 to 6322731 Compare August 20, 2022 20:27

oscargus commented Aug 20, 2022

Copy link
Copy Markdown
Member Author

I've fixed the extends as well now.

oscargus commented Aug 20, 2022
View reviewed changes
Comment thread lib/matplotlib/tests/test_colorbar.py
def test_colorbar_contourf_extend_patches():
params = [
('both', 5, ['\\', '//']),
('min', 7, ['+']),

oscargus Aug 20, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

oscargus commented Aug 20, 2022
View reviewed changes
Comment thread lib/matplotlib/tests/test_colorbar.py Outdated
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)

oscargus Aug 20, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

"Oversized" colorbar so that hatches are visible.

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

jklymak commented Sep 8, 2022

Copy link
Copy Markdown
Member

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

QuLogic reviewed Sep 9, 2022
View reviewed changes
Comment thread lib/matplotlib/tests/test_colorbar.py Outdated
Comment thread lib/matplotlib/tests/test_colorbar.py Outdated
Comment thread lib/matplotlib/tests/test_colorbar.py Outdated

oscargus commented Sep 29, 2022

Copy link
Copy Markdown
Member Author

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

oscargus force-pushed the colorbarhatchrectangle branch from 6322731 to d472042 Compare October 4, 2022 06:43
QuLogic reviewed Oct 4, 2022
View reviewed changes
Comment thread lib/matplotlib/colorbar.py Outdated
Comment thread lib/matplotlib/colorbar.py Outdated
oscargus force-pushed the colorbarhatchrectangle branch from d472042 to f521589 Compare October 5, 2022 07:26
QuLogic reviewed Oct 5, 2022
View reviewed changes
Comment thread lib/matplotlib/colorbar.py
QuLogic reviewed Oct 5, 2022
View reviewed changes
Comment thread lib/matplotlib/colorbar.py Outdated
oscargus force-pushed the colorbarhatchrectangle branch from f521589 to 9576ab3 Compare October 6, 2022 07:43
QuLogic merged commit 60d3d29 into matplotlib:main Oct 6, 2022
meeseeksmachine mentioned this pull request Oct 6, 2022
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 6, 2022
Backport PR matplotlib#23684: Fix rectangle and hatches for colorbar
fd4bd80
QuLogic added a commit that referenced this pull request Oct 6, 2022
Merge pull request #24103 from meeseeksmachine/auto-backport-of-pr-23…
837ae56 
…684-on-v3.6.x

Backport PR #23684 on branch v3.6.x (Fix rectangle and hatches for colorbar)
oscargus deleted the colorbarhatchrectangle branch October 6, 2022 10:40
ksunden mentioned this pull request Feb 20, 2023
Merged
6 tasks
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic approved these changes
jklymak jklymak approved these changes

Assignees

No one assigned

Labels

topic: color/colorbar topic: contour topic: hatch

Projects

None yet

Milestone

v3.6.1

Development

Successfully merging this pull request may close these issues.

[Bug]: Horizontal colorbars drawn incorrectly with hatches

3 participants

Back | FazBrowse Home | New Git URL