8000 Fix a crash when saving to PDF or SVG with contour hatching. by mdboom · Pull Request #891 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 2 commits into from
May 27, 2012
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix a crash when saving to PDF or SVG with contour hatching.
  • Loading branch information
mdboom committed May 23, 2012
commit 20a6e60e5779bcfd6bebf4b91e696f782e4ac008
3 changes: 3 additions & 0 deletions lib/matplotlib/backends/backend_pdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,9 @@ def alphaState(self, alpha):
return name

def hatchPattern(self, hatch_style):
# The colors may come in as numpy arrays, which aren't hashable
hatch_style = (tuple(hatch_style[0]), tuple(hatch_style[1]), hatch_style[2])
Copy link
Member

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 what hatch is. Actually, what is hatch :-)? Is it always immutable or hashable?

Copy link
Member Author

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.

Copy link
Member

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

Copy link
Member Author

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)

Copy link
Member

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.


pattern = self.hatchPatterns.get(hatch_style, None)
if pattern is not None:
return pattern
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_svg.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ def _get_hatch(self, gc, rgbFace):
"""
Create a new hatch pattern
"""
dictkey = (gc.get_hatch(), rgbFace, gc.get_rgb())
dictkey = (gc.get_hatch(), tuple(rgbFace), tuple(gc.get_rgb()))
oid = self._hatchd.get(dictkey)
if oid is None:
oid = self._make_id(u'h', dictkey)
Expand Down
Binary file not shown.
15 changes: 15 additions & 0 deletions lib/matplotlib/tests/test_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,21 @@ def test_hist_log():
ax.set_xticks([])
ax.set_yticks([])

@image_comparison(baseline_images=['contour_hatching'])
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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=['-', '/', '\\', '//'],
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

The 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)
0