[matplotlib-devel] Arc requires explicitly setting fill=False?

2009-05-24 Thread Tony S Yu
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


--
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
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel


Re: [matplotlib-devel] Arc requires explicitly setting fill=False?

2009-05-24 Thread Eric Firing
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 +
+++ b/matplotlib/lib/matplotlib/patches.py  Mon May 25 00:16:44 2009 +
@@ -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
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel


--
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
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel


Re: [matplotlib-devel] Arc requires explicitly setting fill=False?

2009-05-24 Thread John Hunter
On Sun, May 24, 2009 at 7:20 PM, Eric Firing  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.

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
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel


Re: [matplotlib-devel] Arc requires explicitly setting fill=False?

2009-05-24 Thread Eric Firing
John Hunter wrote:
> On Sun, May 24, 2009 at 7:20 PM, Eric Firing  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
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel