-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix a crash when saving to PDF or SVG with contour hatching. #891
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -660,6 +660,21 @@ def test_hist_log(): | |
ax.set_xticks([]) | ||
ax.set_yticks([]) | ||
|
||
@image_comparison(baseline_images=['contour_hatching']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this only produce a pdf? (there was no png or svg added in the commit if not) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's odd. I didn't mean to not include those files. Adding now... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of interest, does not including the baseline images produce a known error rather than a full blown error in the test suite? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It produces a full-blown error, but the new non-baseline images are produced. I consider "known errors" to be things we expect to fail in a certain environmental situation (i.e. missing an optional dependency). Missing the baseline images means that matplotlib didn't get installed correctly (or it's a new test). |
||
def test_contour_hatching(): | ||
x = np.linspace(-3, 5, 150).reshape(1, -1) | ||
y = np.linspace(-3, 5, 120).reshape(-1, 1) | ||
z = np.cos(x) + np.sin(y) | ||
|
||
# we no longer need x and y to be 2 dimensional, so flatten them. | ||
x, y = x.flatten(), y.flatten() | ||
|
||
fig = plt.figure() | ||
ax = fig.add_subplot(111) | ||
cs = ax.contourf(x, y, z, hatches=['-', '/', '\\', '//'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this trigger the exception your fixing? Might be worth making this a numpy array from the off? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What triggers the exception is having colors for the hatch that come from a collection or colormap -- in which case each rgb triple is a numpy array, not something hashable as was previously assumed by the PDF and SVG backends. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should also note -- it doesn't make sense to convert to tuples outside of the backends because the Agg backend benefits from having the colors as a Numpy array. |
||
cmap=plt.get_cmap('gray'), | ||
extend='both', alpha=0.5) | ||
|
||
if __name__=='__main__': | ||
import nose | ||
nose.runmodule(argv=['-s','--with-doctest'], exit=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to look up what hatch_style actually was. Maybe its worth stating in the docstring that it's
hatch_style = (self._rgb, self._fillcolor, hatch)
and whathatch
is. Actually, what ishatch
:-)? Is it always immutable or hashable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No -- many of the backends don't require it to be hashable. This change is really an implementation detail of the PDF and SVG backends.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I was referring to
hatch_style[2]
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hatch_style[2] should always be a string (and thus hashable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Mike. As I said, I think this line would benefit from a comment indicating what hatch_style is likely to be.