Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
jklymak wants to merge2 commits intomatplotlib:main
base:main
Choose a base branch
Loading
fromjklymak:mnt-fix-arc
Open
Show file tree
Hide file tree
Changes fromall 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
112 changes: 78 additions & 34 deletionslib/matplotlib/path.py
View file
Open in desktop
Original file line numberDiff line numberDiff 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` fortheunit circle arc from angles *theta1* to
*theta2* (in degrees).
Return a `Path` fora counter-clockwiseunit 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*)ofthearc.Ifthearcspansmorethan360degrees,it
willbewrappedtofitwithintherangefrom*theta1*to*theta1*+
360,provided*wrap*isTrue.Thearcisdrawncounter-clockwise
(*theta2*)ofthearc.Ifthearcspansmorethan360degrees,it
willbewrappedtostartat*theta1*andhaveatotalextent<=360degrees,
provided*wrap*isTrue.Thearcisdrawncounter-clockwise

I think this is a bit clearer than talking abouttheta1 + 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
IfTrue,thearciswrappedtofitbetween*theta1*and*theta1*+
360degrees.IfFalse,thearcisnotwrapped.Thearcwillbe
IfTrue,thearciswrappedtostartat*theta1*andhaveanextent<=360degrees.
IfFalse,thearcisnotwrapped.Thearcwillbe

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
MemberAuthor

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.

Does2 * 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 DownExpand 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 deletionslib/matplotlib/path.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff 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
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.
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 deletionslib/matplotlib/tests/test_path.py
View file
Open in desktop
Original file line numberDiff line numberDiff 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
MemberAuthor

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 callingarc 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 onax[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

[8]ページ先頭

©2009-2025 Movatter.jp