-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Close clipped paths #4186
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
Close clipped paths #4186
Conversation
one of the failures has to do with the recent fix to trifinder. Did numpy On Tue, Mar 3, 2015 at 7:00 PM, Michael Droettboom <notifications@github.com
|
extend='both',) | ||
plt.colorbar(im,cax=ax4,cmap=cmap,orientation='horizontal', | ||
extend='both',extendrect=True) | ||
plt.colorbar(im,cax=ax5,cmap=cmap,orientation='horizontal', |
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.
this test is failing PEP8 because of the lack of spaces around the commas.
Would it be worth making the line width on the tests very large (given that we don't have exact tests). |
ax4 = fig.add_axes([0.05, 0.25, 0.9, 0.1]) | ||
ax5 = fig.add_axes([0.05, 0.05, 0.9, 0.1]) | ||
|
||
cmap = cm.jet |
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.
Maybe the image files would be smaller if the map had fewer colors?
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.
Also in the spirit of doing-in-jet should we use a different color map ;)
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.
How about changing the cmap to this?:
...
from matplotlib.colors import ListedColormap
cmap = ListedColormap([(.25,.25,.25,1.0),(.5,.5,.5,1.0),(.75,.75,.75,1.0)])
cmap.set_under('w')
cmap.set_over('w')
im = ax1.pcolormesh(np.linspace(0,10,16).reshape((4,4)),cmap=cmap)
...
df8eebd
to
5b82af6
Compare
I think everything above is now addressed. |
@tacaswell: Yes, the default join style appears to be different (miter vs. bevel). I'd rather treat that as a separate issue, as that's likely to affect many more test cases. |
The clipping of the svg image is dependent on the viewer. On my windows machine, they look different in Chrome versus Firefox. |
Edit: I'm sorry, I said that backwards. Inkscape is the same as Chrome. Firefox is the oddball. |
The difference we are seeing is that the stroke width and angle are high enough that the miter limit is going into effect. Here's a good explanation of miter limit: http://tutorials.jenkov.com/svg/stroke.html#stroke-miterlimit The default miter limit is somewhat dependent on the renderer (it's specified in the SVG standard, for example, but I'm not certain every implementation follows it), and what we should be doing is setting it explicitly to get uniform behavior. Ideally, we'd add an API for the user to set the miter limit explicitly, but we've never had that before (i.e. it's not a regression to not have it), and doing so is sort of beyond the scope of this PR. Instead, I think for now we just set it to some reasonable maximum (effectively disabling it), since if the user wants the miters to be limited, they can always set the join style to bevel anyway (not exactly the same thing, of course, but good enough for now). Now that I have a better understanding of what's going on here, I'll probably just add the fix to this PR (since the test here serves as a good test for the other). Stay tuned... |
1d97440
to
a93f4b9
Compare
Ok. The miter limit change is now included. It did not affect any existing tests, other than the new one in this PR. |
a93f4b9
to
3b63c19
Compare
I've updated the test images so the miter points don't extend beyond the edge of the image. |
@mdboom, thank you for taking care of this so quickly. It looks like it needs to be cherry-picked to the color_overhaul branch, correct? I would be more comfortable with you or @tacaswell doing that; I'm shaky with that git operation. |
@mdboom , thank you. I installed master on my Linux machine and your fix works great. Again thanks for the fast work. |
@mdboom This touches code that was greatly changed as part of the cxx refactoring. Can you have a look at back-porting this? |
Fix #4185.