patches.polygon: fix bug in handling of path closing, #1018. #1071
Conversation
This is a modification of that pull request, with a test added.
| return self._path.vertices | ||
|
|
||
| def set_xy(self, xy): | ||
| xy = np.asarray(xy, np.float_) |
There was a problem hiding this comment.
Is it necessary to explicitly cast this dtype to be floats? I'm not against it, I just don't see it done elsewhere.
There was a problem hiding this comment.
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.
|
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 Additionally, I believe the test_patches module name needs to be put in |
|
@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. |
patches.polygon: fix bug in handling of path closing, #1018.
This is a modification of that pull request, with a test added, both based on the comments on #1018