matplotlib-devel Mailing List for matplotlib

Ryan May wrote:
> Hi,
> 
> I just hit a bug in PatchCollection, at line 891 in collections.py:
> 
>          if match_original:
>              def determine_facecolor(patch):
>                  if patch.fill():
>                      return patch.get_facecolor()
>                  return [0, 0, 0, 0]
> 
> The problem is that patch.fill is a boolean attribute, not a function. 
> (Or at least it is for polygons and circles.)  You only hit this if you 
> override the default and specify match_original=True.
> 
> Ryan
> 

Fixed in 5764.  Thanks, Ryan.

Eric

Hi,

I don't think line 96 of collections.py is doing anything:

         if offsets is not None:
             offsets = np.asarray(offsets, np.float_)
             if len(offsets.shape) == 1:
                 offsets = offsets[np.newaxis,:]  # Make it Nx2.
             if transOffset is not None:
                 Affine2D = transforms.Affine2D      <----*********
                 self._offsets = offsets
                 self._transOffset = transOffset
             else:
                 self._uniform_offsets = offsets

         self._pickradius = pickradius
         self.update(kwargs)

This is in __init__ for Collection, which ends with the code I've pasted 
here.  It doesn't appear that Affine2D is used and is probably left over 
cruft.

Ryan

-- 
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma

Hi,

I just hit a bug in PatchCollection, at line 891 in collections.py:

         if match_original:
             def determine_facecolor(patch):
                 if patch.fill():
                     return patch.get_facecolor()
                 return [0, 0, 0, 0]

The problem is that patch.fill is a boolean attribute, not a function. 
(Or at least it is for polygons and circles.)  You only hit this if you 
override the default and specify match_original=True.

Ryan

-- 
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma

On Sat, Jul 12, 2008 at 4:31 PM, Eric Firing <ef...@ha...> wrote:

> and I am suggesting that this handling of None as a default should
> always be moved to a setter if one exists.  That way, a user can always
> get a property back to its default value by setting it to None.
>
> Does anyone immediately see any problem or disadvantage in this?  (I
> have not looked through the code carefully myself, so I could easily be
> missing something.)

I think localizing the logic into the setters is mostly a win-win
proposition, but there are a couple of points you raised that I want
to comment on.   I consider our overloading of None to mean "do the
default, use rc" something of a wart, since it would be more properly
handled with a proper object or special value, like a *class
RCDefault* instance or a string literal 'rcdefault'.   The None
handling may be a wart we are committed to because of legacy code, or
it may be something we will change, but we should recognize it as a
somewhat non-intuitive wart.  I say "somewhat" because None is used in
other python codes as a default sentinel for mutable keyword arguments
(eg, lists and dicts) but otherwise I do not find it terribly obvious.

The 2nd comment is that I don't find this use case compelling.  Using
None in a setter to mean "restore the rc default" may be consistent
with the __init__ handling, but it is not something that anyone has
ever asked for.  In fact, in the use case that prompted this fix, the
OP intended 'None' rather than None, so the fact that mpl raised on
None rather than reverting to the rc default might be considered a
feature rather than a bug.

So basically, I'm +0 on this proposal.  Better than what we have, not ideal.

JDH