patches.polygon: fix bug in handling of path closing, #1018. #1071

merged 3 commits into from Aug 12, 2012

3 participants

Matplotlib Developers member

This is a modification of that pull request, with a test added, both based on the comments on #1018

@efiring efiring patches.polygon: fix bug in handling of path closing, #1018.
This is a modification of that pull request, with a test added.
@WeatherGod WeatherGod and 1 other commented on an outdated diff Aug 12, 2012
@@ -783,20 +779,22 @@ def get_closed(self):
def set_closed(self, closed):
if self._closed == bool(closed):
- self._closed = closed
- xy = self._get_xy()
- if closed:
+ self._closed = bool(closed)
+ self.set_xy(self.get_xy())
+ def get_xy(self):
+ return self._path.vertices
+ def set_xy(self, xy):
+ xy = np.asarray(xy, np.float_)
WeatherGod Aug 12, 2012 Matplotlib Developers member

Is it necessary to explicitly cast this dtype to be floats? I'm not against it, I just don't see it done elsewhere.

efiring Aug 12, 2012 Matplotlib Developers member

The cast was already in the code, but in fact it is not necessary because xy is passed to the Path initializer, which performs the cast. I can take it out.

Matplotlib Developers member

One thing I would like to see more of is to have a docstring in the top of a test module explaining its purpose so that future developers know where to put their tests. In this case, what other tests do you think might go in the test_patches module in the future?

Additionally, I believe the test_patches module name needs to be put in matplotlib/ (see

Matplotlib Developers member

@pelson, I think that there are two primary categories of test modules: those that hold miscellaneous tests from a corresponding mpl module (e.g. test_axes), and those that hold tests of tricky behavior (e.g., test_simplification), sophisticated enough to warrant a separate module. So the purpose of test_patches is to hold the present test, and to provide a home for any additional such tests that are added as bugs are identified and fixed. This is all just my impression of how the use of nosetests has evolved; I don't think the strategy was ever fully planned and documented in detail.

@efiring efiring merged commit 8a5afe3 into matplotlib:master Aug 12, 2012
@efiring efiring deleted the efiring:close_polygon branch May 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment