Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from
+30 −0

Conversation

pharshalp
Copy link
Contributor

@pharshalp pharshalp commented Sep 23, 2018 •
edited
Loading

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:
test

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

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 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 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 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
Copy link
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
Copy link
Member

jklymak commented Sep 23, 2018

@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 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
Copy link
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.

@dstansby dstansby added this to the v3.1 milestone Sep 24, 2018
@pharshalp
Copy link
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
Copy link
Member

efiring commented Sep 25, 2018

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
Copy link
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
Copy link
Member

jklymak commented Feb 26, 2019

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

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

efiring commented Sep 11, 2020

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

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

efiring commented May 13, 2021

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 efiring closed this May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jklymak jklymak jklymak left review comments

Assignees

@efiring 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
@pharshalp @dopplershift @jklymak @efiring @tacaswell @QuLogic @dstansby