Skip to content

Ensure that Path.arc works for any full circle. #8994

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 4 commits into from
Aug 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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
21 changes: 15 additions & 6 deletions lib/matplotlib/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,10 @@ def arc(cls, theta1, theta2, n=None, is_wedge=False):
Return an arc on the unit circle from angle
*theta1* to angle *theta2* (in degrees).

*theta2* is unwrapped to produce the shortest arc within 360 degrees.
That is, if *theta2* > *theta1* + 360, the arc will be from *theta1* to
*theta2* - 360 and not a full circle plus some extra overlap.

If *n* is provided, it is the number of spline segments to make.
If *n* is not provided, the number of spline segments is
determined based on the delta between *theta1* and *theta2*.
Expand All @@ -864,14 +868,15 @@ def arc(cls, theta1, theta2, n=None, is_wedge=False):
polylines, quadratic or cubic Bezier curves
<http://www.spaceroots.org/documents/ellipse/index.html>`_.
"""
theta1, theta2 = np.deg2rad([theta1, theta2])

twopi = np.pi * 2.0
halfpi = np.pi * 0.5

eta1 = np.arctan2(np.sin(theta1), np.cos(theta1))
eta2 = np.arctan2(np.sin(theta2), np.cos(theta2))
eta2 -= twopi * np.floor((eta2 - eta1) / twopi)
eta1 = theta1
eta2 = theta2 - 360 * np.floor((theta2 - theta1) / 360)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use floor instead of // ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few edge cases where it's not the same, but I'm not sure if that's the reason. This is just a rearrangment of existing code changed from radians to degrees (because the trig is not necessary here.)

# Ensure 2pi range is not flattened to 0 due to floating-point errors,
# but don't try to expand existing 0 range.
if theta2 != theta1 and eta2 <= eta1:
eta2 += 360
eta1, eta2 = np.deg2rad([eta1, eta2])

# number of curve segments to make
if n is None:
Expand Down Expand Up @@ -930,6 +935,10 @@ def wedge(cls, theta1, theta2, n=None):
Return a wedge of the unit circle from angle
*theta1* to angle *theta2* (in degrees).

*theta2* is unwrapped to produce the shortest wedge within 360 degrees.
That is, if *theta2* > *theta1* + 360, the wedge will be from *theta1*
to *theta2* - 360 and not a full circle plus some extra overlap.

If *n* is provided, it is the number of spline segments to make.
If *n* is not provided, the number of spline segments is
determined based on the delta between *theta1* and *theta2*.
Expand Down
Binary file modified lib/matplotlib/tests/baseline_images/test_patches/wedge_range.pdf
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
117 changes: 56 additions & 61 deletions lib/matplotlib/tests/baseline_images/test_patches/wedge_range.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 12 additions & 0 deletions lib/matplotlib/tests/test_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,15 @@ def test_path_deepcopy():
path2 = Path(verts, codes)
copy.deepcopy(path1)
copy.deepcopy(path2)


@pytest.mark.parametrize('offset', range(-720, 361, 45))
def test_full_arc(offset):
low = offset
high = 360 + offset

path = Path.arc(low, high)
mins = np.min(path.vertices, axis=0)
maxs = np.max(path.vertices, axis=0)
np.testing.assert_allclose(mins, -1)
assert np.allclose(maxs, 1)