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

FIX: pcolor/pcolormesh honour edgecolors kwarg when facecolors is set 'none' by pharshalp · Pull Request #12226 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

FIX: pcolor/pcolormesh honour edgecolors kwarg when facecolors is set 'none' - #12226

Closed
pharshalp wants to merge 1 commit into
matplotlib:masterfrom
pharshalp:fix_pcolor_pcolormesh_edgecolor
ConversationCommits (1)ChecksFiles changed
Closed

FIX: pcolor/pcolormesh honour edgecolors kwarg when facecolors is set 'none'#12226
pharshalp wants to merge 1 commit into
matplotlib:masterfrom
pharshalp:fix_pcolor_pcolormesh_edgecolor

Conversation

pharshalp commented Sep 23, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Closes #1302

This fixes the issue #1302 [edgecolor(s) being ignored by pcolor/pcolormesh when facecolor(s) is set to 'none'].

From a quick look at the discussion in #1302 and looking at the underlying code, I think this PR should work. NOTE: I am not entirely familiar with how the Collection object is supposed to behave. If this PR fails the tests then I will close this PR and dig deeper to find the correct approach. (all the tests passed!).

Example:

import matplotlib.pyplot as plt
import numpy as np

plt.pcolor(np.arange(12).reshape(4, 3), edgecolors='red', facecolors='none')
plt.show()

Current outcome:

Correct outcome (after applying the fix in this PR):

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • [ ] New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • [ ] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [ ] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

pharshalp changed the title [WIP] pcolor/pcolormesh bug #1302 (edgecolors being ignored by pcolor/pcolormesh when facecolors is set 'none'). pcolor/pcolormesh bug #1302 (edgecolors being ignored by pcolor/pcolormesh when facecolors is set 'none'). Sep 23, 2018
pharshalp changed the title pcolor/pcolormesh bug #1302 (edgecolors being ignored by pcolor/pcolormesh when facecolors is set 'none'). pcolor/pcolormesh bug #1302 (edgecolors being ignored when facecolors is set 'none'). Sep 23, 2018
pharshalp changed the title pcolor/pcolormesh bug #1302 (edgecolors being ignored when facecolors is set 'none'). Fixes #1302 (pcolor/pcolormesh bug - edgecolors being ignored when facecolors is set 'none'). Sep 23, 2018

dopplershift commented Sep 23, 2018

Copy link
Copy Markdown
Contributor

Would you be able to turn your example code above into an image test so that we don't break this in the future?

jklymak commented Sep 23, 2018

Copy link
Copy Markdown
Member

@pharslap, looks great. I took the liberty of editing your title - no need to ref original issue, and description of fix is most helpful...

jklymak changed the title Fixes #1302 (pcolor/pcolormesh bug - edgecolors being ignored when facecolors is set 'none'). FIX: pcolor/pcolormesh honour edgecolors kwarg when facecolors is set 'none' Sep 23, 2018

pharshalp commented Sep 23, 2018

Copy link
Copy Markdown
Contributor Author

Would you be able to turn your example code above into an image test so that we don't break this in the future?

@dopplershift I have added a test that doesn't use image comparison.

jklymak reviewed Sep 23, 2018
View reviewed changes
Comment thread lib/matplotlib/tests/test_image.py Outdated
dstansby added this to the v3.1 milestone Sep 24, 2018

pharshalp commented Sep 25, 2018

Copy link
Copy Markdown
Contributor Author

@WeatherGod @efiring Could you please take a look at this? Tagging you since you were involved in the discussion on the issue. Thanks!

efiring commented Sep 25, 2018

Copy link
Copy Markdown
Member

I'm pretty sure this is not the right solution. It looks like there is a simple underlying bug that this is not addressing. Investigating...

pharshalp commented Sep 26, 2018

Copy link
Copy Markdown
Contributor Author

I'm pretty sure this is not the right solution. It looks like there is a simple underlying bug that this is not addressing. Investigating...

Ok. Thanks for taking a look!

jklymak commented Feb 26, 2019

Copy link
Copy Markdown
Member

@efiring - still investigation 😉 I'll assume this can wait for 3.2...

jklymak modified the milestones: v3.1.0, v3.2.0 Feb 26, 2019
efiring self-assigned this May 23, 2019
tacaswell modified the milestones: v3.2.0, v3.3.0 Sep 5, 2019
QuLogic modified the milestones: v3.3.0, v3.4.0 May 2, 2020

efiring commented Sep 11, 2020

Copy link
Copy Markdown
Member

Sorry for the long delay. I will try to take care of this as soon as #6268 goes in.

efiring mentioned this pull request Sep 14, 2020
Merged
5 of 7 tasks
jklymak marked this pull request as draft September 28, 2020 19:46
QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021

efiring commented May 13, 2021

Copy link
Copy Markdown
Member

This is superseded by #18480. Thank you for the contribution, @pharshalp. Although your PR was not merged, it led to a different solution to the problem.

efiring closed this May 13, 2021
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

jklymak jklymak left review comments

Assignees

efiring

Labels

topic: collections and mappables

Projects

None yet

Milestone

v3.5.0

Development

Successfully merging this pull request may close these issues.

pcolormesh bug: edgecolor ignored

7 participants

Back | FazBrowse Home | New Git URL