Skip to content

FIX: update arc to do better rounding and allow wrap #29962

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

FIX: update arc to do better rounding and allow wrap #29962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14d5441
FIX: update arc to do better rounding and allow wrap
jklymak Apr 24, 2025
a4d8129
TST
jklymak Apr 24, 2025
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
112 changes: 78 additions & 34 deletions lib/matplotlib/path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,36 +949,70 @@
return cls._unit_circle_righthalf

@classmethod
def arc(cls, theta1, theta2, n=None, is_wedge=False):
def arc(cls, theta1, theta2, n=None, is_wedge=False, wrap=True):
"""
Return a `Path` for the unit circle arc from angles *theta1* to
*theta2* (in degrees).
Return a `Path` for a counter-clockwise unit circle arc from angles
*theta1* to *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*.
Parameters
----------
theta1, theta2 : float
The angles (in degrees) defining the start (*theta1*) and end
(*theta2*) of the arc. If the arc spans more than 360 degrees, it
will be wrapped to fit within the range from *theta1* to *theta1* +
360, provided *wrap* is True. The arc is drawn counter-clockwise
Comment on lines +961 to +963
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(*theta2*) of the arc. If the arc spans more than 360 degrees, it
will be wrapped to fit within the range from *theta1* to *theta1* +
360, provided *wrap* is True. The arc is drawn counter-clockwise
(*theta2*) of the arc. If the arc spans more than 360 degrees, it
will be wrapped to start at *theta1* and have a total extent <= 360 degrees,
provided *wrap* is True. The arc is drawn counter-clockwise

I think this is a bit clearer than talking about theta1 + 360?

from *theta1* to *theta2*. For instance, if *theta1* =90 and
*theta2* = 70, the resulting arc will span 320 degrees.

n : int, optional
The number of spline segments to make. If not provided, the number
of spline segments is determined based on the delta between
*theta1* and *theta2*.

is_wedge : bool, default: False
If True, the arc is a wedge. The first vertex is the center of the
wedge, the second vertex is the start of the arc, and the last
vertex is the end of the arc. The wedge is closed with a line
segment to the center of the wedge. If False, the arc is a
polyline. The first vertex is the start of the arc, and the last
vertex is the end of the arc. The arc is closed with a line
segment to the start of the arc. The wedge is not closed with a
line segment to the start of the arc.

wrap : bool, default: True
If True, the arc is wrapped to fit between *theta1* and *theta1* +
360 degrees. If False, the arc is not wrapped. The arc will be
Comment on lines +983 to +984
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If True, the arc is wrapped to fit between *theta1* and *theta1* +
360 degrees. If False, the arc is not wrapped. The arc will be
If True, the arc is wrapped to start at *theta1* and have an extent <= 360 degrees.
If False, the arc is not wrapped. The arc will be

drawn from *theta1* to *theta2*.

Masionobe, L. 2003. `Drawing an elliptical arc using
polylines, quadratic or cubic Bezier curves
<https://web.archive.org/web/20190318044212/http://www.spaceroots.org/documents/ellipse/index.html>`_.
Notes
-----
The arc is approximated using cubic Bézier curves, as described in
Masionobe, L. 2003. `Drawing an elliptical arc using polylines,
quadratic or cubic Bezier curves
<https://web.archive.org/web/20190318044212/http://www.spaceroots.org/documents/ellipse/index.html>`_.
"""
halfpi = np.pi * 0.5

eta1 = theta1
eta2 = theta2 - 360 * np.floor((theta2 - theta1) / 360)
# 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
if wrap:
# Wrap theta2 to 0-360 degrees from theta1.
eta2 = np.mod(theta2 - theta1, 360.0) + theta1
# Ensure 360-deg 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
else:
eta2 = theta2
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this new case (wrap=False) needs a test. Probably not a figure test, but something that checks that an arc is drawn with an extent > 360 deg would be good.

eta1, eta2 = np.deg2rad([eta1, eta2])

# number of curve segments to make
if n is None:
n = int(2 ** np.ceil((eta2 - eta1) / halfpi))
if np.abs(eta2 - eta1) <= 2.2 * np.pi:
Copy link
Member

Choose a reason for hiding this comment

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

Why 2.2pi here?

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's exactly 2pi some tests fail. 2.2 pi doesn't really matter because it deals with pi/2 increments. I could make it 2.25 pi?

Copy link
Member

Choose a reason for hiding this comment

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

Does 2 * np.pi + 1e-3 or something similar work? Writing it like this makes it more obvious that the intention is 2pi + a tiny bit for numerical reasons.

# this doesn't need to grow exponentially, but we have left
# this way for back compatibility
n = int(2 ** np.ceil(2 * np.abs(eta2 - eta1) / np.pi))
else:
# this will grow linearly if we allow wrapping arcs:
n = int(2 * np.ceil(2 * np.abs(eta2 - eta1) / np.pi))
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is missing a test too.

if n < 1:
raise ValueError("n must be >= 1 or None")

Expand Down Expand Up @@ -1028,22 +1062,32 @@
return cls(vertices, codes, readonly=True)

@classmethod
def wedge(cls, theta1, theta2, n=None):
def wedge(cls, theta1, theta2, n=None, wrap=True):
"""
Return a `Path` for the unit circle wedge from angles *theta1* to
*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*.
Return a `Path` for a counter-clockwise unit circle wedge from angles
*theta1* to *theta2* (in degrees).

See `Path.arc` for the reference on the approximation used.
"""
return cls.arc(theta1, theta2, n, True)
Parameters
----------
theta1, theta2 : float
The angles (in degrees) defining the start (*theta1*) and end
(*theta2*) of the arc. If the arc spans more than 360 degrees, it
will be wrapped to fit within the range from *theta1* to *theta1* +
360, provided *wrap* is True. The arc is drawn counter-clockwise
from *theta1* to *theta2*. For instance, if *theta1* =90 and
*theta2* = 70, the resulting arc will span 320 degrees.

n : int, optional
The number of spline segments to make. If not provided, the number
of spline segments is determined based on the delta between
*theta1* and *theta2*.

wrap : bool, default: True
If True, the arc is wrapped to fit between *theta1* and *theta1* +
360 degrees. If False, the arc is not wrapped. The arc will be
drawn from *theta1* to *theta2*.
"""
return cls.arc(theta1, theta2, n, wedge=True, wrap=wrap)

Check warning on line 1090 in lib/matplotlib/path.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/path.py#L1090 

Added line #L1090 was not covered by tests

@staticmethod
@lru_cache(8)
Expand Down
8 changes: 6 additions & 2 deletions lib/matplotlib/path.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,14 @@ class Path:
def unit_circle_righthalf(cls) -> Path: ...
@classmethod
def arc(
cls, theta1: float, theta2: float, n: int | None = ..., is_wedge: bool = ...
cls, theta1: float, theta2: float, n: int | None = ...,
is_wedge: bool = ..., wrap: bool = ...
) -> Path: ...
@classmethod
def wedge(cls, theta1: float, theta2: float, n: int | None = ...) -> Path: ...
def wedge(
cls, theta1: float, theta2: float, n: int | None = ...,
wrap: bool = ...
) -> Path: ...
@overload
@staticmethod
def hatch(hatchpattern: str, density: float = ...) -> Path: ...
Expand Down
Binary file added lib/matplotlib/tests/baseline_images/test_path/arc_close360.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added lib/matplotlib/tests/baseline_images/test_path/arc_wrap_false.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
37 changes: 37 additions & 0 deletions lib/matplotlib/tests/test_path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,12 +471,49 @@ def test_full_arc(offset):
high = 360 + offset

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


@image_comparison(['arc_close360'], style='default', remove_text=True,
extensions=['png'])
def test_arc_close360():
Copy link
Member

Choose a reason for hiding this comment

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

Can you do this as a compare images test, instead of a figure test that adds a new figure?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how to make the requisite arcs without calling arc so I'm not sure what compare figure test would do? I could just make a full circle at exactly 360 degrees, but I'm not sure what that is testing.

Copy link
Member

Choose a reason for hiding this comment

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

For this test, can you compare what you're generating on ax[0] and ax[1] against each other, and then compare what you're plotting on ax[2] against an empty axes?

_, ax = plt.subplots(ncols=3)
ax[0].add_patch(patches.PathPatch(Path.arc(theta1=-90 - 1e-14, theta2=270)))
#ax[0].set_title("arc(-90-1e-14, 270), should be a circle")
ax[1].add_patch(patches.PathPatch(Path.arc(theta1=-90, theta2=270)))
#ax[1].set_title("arc(-90, 270), is a circle")
ax[2].add_patch(patches.PathPatch(Path.arc(theta1=-90 - 1e-14, theta2=-90)))
#ax[2].set_title("arc(-90, -90-1e-14), should not be a circle")
for a in ax:
a.set_xlim(-1, 1)
a.set_ylim(-1, 1)
a.set_aspect("equal")


@image_comparison(['arc_wrap_false'], style='default', remove_text=True,
extensions=['png'])
def test_arc_wrap_false():
_, ax = plt.subplots(2, 2)
ax = ax.flatten()
ax[0].add_patch(patches.PathPatch(Path.arc(theta1=10, theta2=20,
is_wedge=True, wrap=True)))
ax[1].add_patch(patches.PathPatch(Path.arc(theta1=10, theta2=380,
is_wedge=True, wrap=True)))
ax[2].add_patch(patches.PathPatch(Path.arc(theta1=10, theta2=20,
is_wedge=True, wrap=False)))
ax[3].add_patch(patches.PathPatch(Path.arc(theta1=10, theta2=740,
is_wedge=True, wrap=False)))
for a in ax:
a.set_xlim(-1, 1)
a.set_ylim(-1, 1)
a.set_aspect("equal")



def test_disjoint_zero_length_segment():
this_path = Path(
np.array([
Expand Down
Loading