Currently, Arc in matplotlib.patches requires that it be called with kwarg ``fill=False``. Was this behavior intentional? The code suggests that a default value was left out of the kwarg lookup.
I've attached a simple patch to fix this (it still fails when fill set to True).
Cheers,
-Tony
Index: lib/matplotlib/patches.py
···
===================================================================
--- lib/matplotlib/patches.py (revision 7137)
+++ lib/matplotlib/patches.py (working copy)
@@ -1189,7 +1189,7 @@
%(Patch)s
"""
- fill = kwargs.pop('fill')
+ fill = kwargs.pop('fill', False)
if fill:
raise ValueError("Arc objects can not be filled")
kwargs['fill'] = False
Tony S Yu wrote:
Currently, Arc in matplotlib.patches requires that it be called with kwarg ``fill=False``. Was this behavior intentional? The code suggests that a default value was left out of the kwarg lookup.
I've attached a simple patch to fix this (it still fails when fill set to True).
Thanks. I committed a slightly different fix. I think this handles all possibilities.
--- a/matplotlib/lib/matplotlib/patches.py Mon May 25 00:00:46 2009 +0000
+++ b/matplotlib/lib/matplotlib/patches.py Mon May 25 00:16:44 2009 +0000
@@ -1189,10 +1189,9 @@
%(Patch)s
"""
- fill = kwargs.get('fill') # returns None if key is absent
+ fill = kwargs.setdefault('fill', False)
if fill:
raise ValueError("Arc objects can not be filled")
- kwargs['fill'] = False
Ellipse.__init__(self, xy, width, height, angle, **kwargs)
Eric
···
Cheers,
-Tony
Index: lib/matplotlib/patches.py
--- lib/matplotlib/patches.py (revision 7137)
+++ lib/matplotlib/patches.py (working copy)
@@ -1189,7 +1189,7 @@
%(Patch)s
"""
- fill = kwargs.pop('fill')
+ fill = kwargs.pop('fill', False)
if fill:
raise ValueError("Arc objects can not be filled")
kwargs['fill'] = False
------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, & iPhoneDevCamp asthey present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://www.creativitycat.com _______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options
Michael can weigh in on this when he has a chance, but my recollection
is that Arc was added to satisfy a JPL reported bug when one zooms
into a small region of an ellipse -- in that case our 4 spline
approximation code was inadequate, and in a heroic burst Michael
provided an 8 spline interpolation limited to the viewport. Ie,
instead of getting 4 splines for the entire ellipse, with his Arc
class you get 8 for the segment in the viewport. As part of this, he
decided it was mostly impossible to fully support filling, or at least
too difficult, so he may have intentionally raised this error. So we
should be careful here, because it may be that simple arcs, those
where everything is in the viewport, work ok with filling, but things
break down when his zoom optimizations are triggered.
JDH
···
On Sun, May 24, 2009 at 7:20 PM, Eric Firing <efiring@...229...> wrote:
Tony S Yu wrote:
Currently, Arc in matplotlib.patches requires that it be called with
kwarg ``fill=False``. Was this behavior intentional? The code suggests
that a default value was left out of the kwarg lookup.
I've attached a simple patch to fix this (it still fails when fill set
to True).
Thanks. I committed a slightly different fix. I think this handles all
possibilities.
John Hunter wrote:
Tony S Yu wrote:
Currently, Arc in matplotlib.patches requires that it be called with
kwarg ``fill=False``. Was this behavior intentional? The code suggests
that a default value was left out of the kwarg lookup.
I've attached a simple patch to fix this (it still fails when fill set
to True).
Thanks. I committed a slightly different fix. I think this handles all
possibilities.
Michael can weigh in on this when he has a chance, but my recollection
is that Arc was added to satisfy a JPL reported bug when one zooms
into a small region of an ellipse -- in that case our 4 spline
approximation code was inadequate, and in a heroic burst Michael
provided an 8 spline interpolation limited to the viewport. Ie,
instead of getting 4 splines for the entire ellipse, with his Arc
class you get 8 for the segment in the viewport. As part of this, he
decided it was mostly impossible to fully support filling, or at least
too difficult, so he may have intentionally raised this error. So we
should be careful here, because it may be that simple arcs, those
where everything is in the viewport, work ok with filling, but things
break down when his zoom optimizations are triggered.
John,
Yes, Arc is a very special-purpose class, and not really a patch at all. Actually, according to the docstrings, the Ellipse is calculated with 8 splines, and Arc is calculated with 8 splines for the viewable portion alone.
The change I made merely made it so that Arc works with no fill kwarg at all, or with fill=False, and as before, it raises an error if fill==True. I suspect this is the behavior Mike intended--I doubt he meant to *require* a kwarg that can take only one value without raising an error--but certainly he can correct me if I am mistaken.
Eric
···
On Sun, May 24, 2009 at 7:20 PM, Eric Firing <efiring@...229...> wrote:
JDH
FYI: The main reason we needed Arc was related to zooming accuracy and performance. We're drawing ellipses on a planetary scale (1000's of km) and then drawing other items (targeting points, error ellipses, etc) in only a small part of the planetary ellipse (1-10 km). Drawing a complete ellipse and then zooming way in to see everything else meant that the accuracy of the ellipse in the area we were looking at is crucial (and the performance of the agg in this mode is terrible). Arc allows us to draw just the part of the planetary ellipse in the area we care about so we get great accuracy and a huge speed increase.
Ted
···
-----Original Message-----
From: Eric Firing [mailto:efiring@…229…]
Sent: Sunday, May 24, 2009 9:59 PM
To: John Hunter
Cc: Tony S Yu; matplotlib development list
Subject: Re: [matplotlib-devel] Arc requires explicitly setting
fill=False?
John Hunter wrote:
> On Sun, May 24, 2009 at 7:20 PM, Eric Firing <efiring@...229...> > wrote:
>> Tony S Yu wrote:
>>> Currently, Arc in matplotlib.patches requires that it be called
with
>>> kwarg ``fill=False``. Was this behavior intentional? The code
suggests
>>> that a default value was left out of the kwarg lookup.
>>>
>>> I've attached a simple patch to fix this (it still fails when fill
set
>>> to True).
>> Thanks. I committed a slightly different fix. I think this handles
all
>> possibilities.
>>
>
> Michael can weigh in on this when he has a chance, but my
recollection
> is that Arc was added to satisfy a JPL reported bug when one zooms
> into a small region of an ellipse -- in that case our 4 spline
> approximation code was inadequate, and in a heroic burst Michael
> provided an 8 spline interpolation limited to the viewport. Ie,
> instead of getting 4 splines for the entire ellipse, with his Arc
> class you get 8 for the segment in the viewport. As part of this, he
> decided it was mostly impossible to fully support filling, or at
least
> too difficult, so he may have intentionally raised this error. So we
> should be careful here, because it may be that simple arcs, those
> where everything is in the viewport, work ok with filling, but things
> break down when his zoom optimizations are triggered.
John,
Yes, Arc is a very special-purpose class, and not really a patch at
all.
Actually, according to the docstrings, the Ellipse is calculated with
8 splines, and Arc is calculated with 8 splines for the viewable
portion
alone.
The change I made merely made it so that Arc works with no fill kwarg
at
all, or with fill=False, and as before, it raises an error if
fill==True. I suspect this is the behavior Mike intended--I doubt he
meant to *require* a kwarg that can take only one value without raising
an error--but certainly he can correct me if I am mistaken.
Eric
>
> JDH
-----------------------------------------------------------------------
-------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity
professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, &
iPhoneDevCamp asthey present alongside digital heavyweights like
Barbarian
Group, R/GA, & Big Spaceship. http://www.creativitycat.com
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options
No virus found in this incoming message.
Checked by AVG - www.avg.com
Version: 8.5.339 / Virus Database: 270.12.37/2130 - Release Date:
05/24/09 07:09:00
Yes -- that is correct. Arc's are unfillable by design -- such a thing might be possible, but it would require some extra work to the code to generate rectilinear lines around the edges of the clipped area.
I think this fix is correct -- the purpose is to warn the user trying to fill an Arc that filling is not possible.
Cheers,
Mike
Eric Firing wrote:
···
John Hunter wrote:
On Sun, May 24, 2009 at 7:20 PM, Eric Firing <efiring@...229...> wrote:
Tony S Yu wrote:
Currently, Arc in matplotlib.patches requires that it be called with
kwarg ``fill=False``. Was this behavior intentional? The code suggests
that a default value was left out of the kwarg lookup.
I've attached a simple patch to fix this (it still fails when fill set
to True).
Thanks. I committed a slightly different fix. I think this handles all
possibilities.
Michael can weigh in on this when he has a chance, but my recollection
is that Arc was added to satisfy a JPL reported bug when one zooms
into a small region of an ellipse -- in that case our 4 spline
approximation code was inadequate, and in a heroic burst Michael
provided an 8 spline interpolation limited to the viewport. Ie,
instead of getting 4 splines for the entire ellipse, with his Arc
class you get 8 for the segment in the viewport. As part of this, he
decided it was mostly impossible to fully support filling, or at least
too difficult, so he may have intentionally raised this error. So we
should be careful here, because it may be that simple arcs, those
where everything is in the viewport, work ok with filling, but things
break down when his zoom optimizations are triggered.
John,
Yes, Arc is a very special-purpose class, and not really a patch at all. Actually, according to the docstrings, the Ellipse is calculated with 8 splines, and Arc is calculated with 8 splines for the viewable portion alone.
The change I made merely made it so that Arc works with no fill kwarg at all, or with fill=False, and as before, it raises an error if fill==True. I suspect this is the behavior Mike intended--I doubt he meant to *require* a kwarg that can take only one value without raising an error--but certainly he can correct me if I am mistaken.
Eric
JDH
------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, & iPhoneDevCamp asthey present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://www.creativitycat.com _______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA