Consistency of the radius argument for Path.points_in_path #2915
Conversation
Updated the points_in_path method to allow negative radii as is implied by the documentation.
|
👍 |
|
Looks good. Any chance of a test case to cover this, otherwise it is likely to get broken again. |
|
There's the test case. The Travis CI build seems to have failed though, the test I added now passes so I can't quite see what I've done wrong? |
|
|
||
| points = [(0.0, 0.0), (1.25, 0.0)] | ||
|
|
||
| assert np.all(path.contains_points(points, radius = -0.5) == [True, False] ) |
There was a problem hiding this comment.
Could you also add a point which would be inside of the unit circle, but not inside of the unit circle with a radius of 0.5 (e.g. (0.9, 0.9)). I may have missed what radius actually does, so if my comment doesn't make any sense, please feel free to enlighten me 😉
There was a problem hiding this comment.
P.S. It's incredibly picky I know, but would you mind removing the spaces between the keyword i.e.:
np.all(path.contains(points, radius=-0.5)...
I think an update to the pep8 tool on pypi has triggered master's tests to fail - this is being dealt with, so don't worry about those. This is looking good IMHO 👍 |
Consistency of the radius argument for Path.points_in_path
Currently the documentation for the Path.points_in_path method implies that you can call it with a negative radius to shrink the path when making the comparison.
The code doesn't behave in that way, so here is a pull request which should fix it. If it does break something then the documentation should be updated.