With matplotlib=2.1.2, when I add an Arc patch to an Axes instance via a PatchCollection, the arc always appear as a complete ellipse, regardless of values I set for theta1 and theta2. When I add the Arc directly via Axes.add_patch, I don't have this problem.
Last time I checked (matplotlib 1.4.0), this wasn't an issue. I haven't bisected to figure out when the problem started.
The problem is that the Arc is essentially an Ellipse; it just has two additional attributes theta1 and theta2 and a completely different draw method. So the Arc only becomes an arc at drawtime.
The PatchCollection does not use the individual patches' draw method, but its own (which draws the paths of the patches).
Possibly it's easier to find a sufficient workaround for this problem than to create a new PatchCollectionWhichAllowsForArcs. @dyf What is the motivation to use a PatchCollection instead adding the arcs individually?
I can certainly add the patches individually. I used a pattern in my code where I create a collection of elements and style them all at once via a PatchCollection, e.g.:
but I can just as easily do this in a loop if that's the current preferred route. Not a big deal.
That said, I wanted to let y'all know that a) this used to work, and b) I think the current behavior is surprising. If you need/want to keep it this way, you might want to update the docs.
This line was removed in #8047 (on 11 Mar 2017). Not sure how it can have been removed before the release but still be in it?!
In any case, reintroducing this line into master will indeed produce arcs instead of ellipses in a PatchCollection; however the bug that #8047 fixed (or at least some other problem that produces wrong arc length) would be reintroduced into the code base.
attn: @dstansby who removed this line in question and @tacaswell who confirmed that "pushing this down into Path.arc is not right".
Does putting that line back in fix this problem, and pass all the tests? Might be worth trying. (sorry, I don't have time to look at this at the moment 😦 )
To be more precise here, I'm considering the following code which should add an Arc between theta1=0 and theta2=60 as a standalone patch as well as inside a PatchCollection.
I think we can just move the logic in the diff for #8047 into into the init rather than the draw. I can’t really think why this would have to be done at draw time, but maybe I’ve not thought it through.
There are two reasons the arc is calculated at drawtime and not in the __init__:
Efficiency: Only calculate the points to draw once it is clear where to draw them.
Units: Only calculate the points once the arc is given an axes with units to live in.
I think it would be possible to sacrifice efficiency (1) and let the total arc be calculated at init; but I have no idea how to do that as long as it's not in an axes (2). I think this second point is what @dstansby stumbled upon when doing the fix, so I thought he might be able to tell a bit more on the reasoning here.
Well, units + PatchCollections bite us again.... This should stay open as something the units MEP should address: How can we pre-specify patches when the final axes will have units?
Activity
ImportanceOfBeingErnest commented on May 18, 2018
The problem is that the Arc is essentially an Ellipse; it just has two additional attributes theta1 and theta2 and a completely different draw method. So the Arc only becomes an arc at drawtime.
The PatchCollection does not use the individual patches' draw method, but its own (which draws the paths of the patches).
Possibly it's easier to find a sufficient workaround for this problem than to create a new PatchCollectionWhichAllowsForArcs.
@dyf What is the motivation to use a PatchCollection instead adding the arcs individually?
dyf commented on May 18, 2018
@ImportanceOfBeingErnest Thanks for the quick response.
I can certainly add the patches individually. I used a pattern in my code where I create a collection of elements and style them all at once via a PatchCollection, e.g.:
https://github.com/AllenInstitute/AllenSDK/blob/master/allensdk/brain_observatory/circle_plots.py#L574
but I can just as easily do this in a loop if that's the current preferred route. Not a big deal.
That said, I wanted to let y'all know that a) this used to work, and b) I think the current behavior is surprising. If you need/want to keep it this way, you might want to update the docs.
Thanks!
jklymak commented on May 18, 2018
I think it’s worth chasing down why it changed.
ImportanceOfBeingErnest commented on May 19, 2018
You're right. My explanation comes short of explaining the issue. The code works as expected in matplotlib 2.0.2.
Version 2.0.2 (released on 5 May 2017) contains the line:
matplotlib/lib/matplotlib/patches.py
Line 1561 in e175a41
This line was removed in #8047 (on 11 Mar 2017). Not sure how it can have been removed before the release but still be in it?!
In any case, reintroducing this line into master will indeed produce arcs instead of ellipses in a PatchCollection; however the bug that #8047 fixed (or at least some other problem that produces wrong arc length) would be reintroduced into the code base.
attn: @dstansby who removed this line in question and @tacaswell who confirmed that "pushing this down into Path.arc is not right".
dstansby commented on May 21, 2018
Does putting that line back in fix this problem, and pass all the tests? Might be worth trying. (sorry, I don't have time to look at this at the moment 😦 )
ImportanceOfBeingErnest commented on May 21, 2018
I did not run any tests, but as said, simply putting self._path = Path.arc(self.theta1, self.theta2) back into the __init__ has two effects:
ImportanceOfBeingErnest commented on May 22, 2018
To be more precise here, I'm considering the following code which should add an Arc between theta1=0 and theta2=60 as a standalone patch as well as inside a PatchCollection.
arc = mpatches.Arc([.5,.5], .5, .8, theta1=0, theta2=60, angle=0, lw=1.5) col = mcol.PatchCollection(patches=[arc], facecolors='none', edgecolors='turquoise', lw=3) ax1.add_patch(arc) ax1.add_collection(col)import numpy as np import matplotlib matplotlib.use("Qt5Agg") import matplotlib.pyplot as plt import matplotlib.patches as mpatches import matplotlib.collections as mcol def angular_grid(ax, cntr=(0,0)): for ang in [0,30,60,90]: x = np.cos(np.deg2rad(ang)) y = np.sin(np.deg2rad(ang)) l = plt.Line2D([cntr[0],cntr[0]+x],[cntr[1],cntr[1]+y], color="0.8", ls="--", zorder=0) ax.add_artist(l) ax.text(cntr[0]+x/2.3, cntr[1]+y/2.3, "{}°".format(ang), color="0.6") def func(): fig1, ax1 = plt.subplots(figsize=(3.2,2.8)) arc = mpatches.Arc([.5,.5], .5, .8, theta1=0, theta2=60, angle=0, lw=1.5) col = mcol.PatchCollection(patches=[arc], facecolors='none', edgecolors='turquoise', lw=3) ax1.add_patch(arc) ax1.add_collection(col) ax1.legend([plt.Line2D([],[],color='k'),plt.Line2D([],[],color='turquoise')], ["Arc", "PatchCollection([Arc])"], loc=3) angular_grid(ax1, [.5,.5]) ax1.set_title("matplotlib {}".format(matplotlib.__version__), fontsize=10) ax1.set_aspect(1) func() plt.show()I'm running this code in several versions:
Matplotlib 2.0.2
Matplotlib 2.2.2
after fix #8047
Matplotlib development version + added self._path in __init__
Adding the line
matplotlib/lib/matplotlib/patches.py
Line 1561 in e175a41
back into master produces:
jklymak commented on May 22, 2018
I think we can just move the logic in the diff for #8047 into into the init rather than the draw. I can’t really think why this would have to be done at draw time, but maybe I’ve not thought it through.
ImportanceOfBeingErnest commented on May 22, 2018
There are two reasons the arc is calculated at drawtime and not in the __init__:
I think it would be possible to sacrifice efficiency (1) and let the total arc be calculated at init; but I have no idea how to do that as long as it's not in an axes (2). I think this second point is what @dstansby stumbled upon when doing the fix, so I thought he might be able to tell a bit more on the reasoning here.
jklymak commented on May 22, 2018
Well, units + PatchCollections bite us again.... This should stay open as something the units MEP should address: How can we pre-specify patches when the final axes will have units?
chibai commented on Jan 20, 2022
Is this bug fixed?
The version of Matplotlib is 3.5.1 now~
timhoffm commented on Jan 25, 2022
No, it's unchanged. This is a fundamental design problem, which nobody has taken the effort to dive into and find a solution.