Issue with Ellipses and PatchCollections

Development matplotlib-devel
#1

Hi,

The following code:

from matplotlib.patches import Ellipse
from matplotlib.collections import PatchCollection
p = PatchCollection([Ellipse((0.5,0.5),0.2,0.1)], match_original=True)

raises the following exception:

Traceback (most recent call last):
  File "test_patches.py", line 5, in <module>
    p = PatchCollection([Ellipse((0.5,0.5),0.2,0.1)], match_original=True)
  File "/Users/tom/Library/Python/2.6/site-packages/matplotlib/collections.py", line 1041, in __init__
    facecolors = [determine_facecolor(p) for p in patches]
  File "/Users/tom/Library/Python/2.6/site-packages/matplotlib/collections.py", line 1037, in determine_facecolor
    if patch.fill:
AttributeError: 'Ellipse' object has no attribute 'fill'

I have submitted a ticket: https://sourceforge.net/tracker/?group_id=80706&atid=560720

Cheers,

Tom

#2

The obvious fix would be to change from patch.fill to
patch.get_fill(). However, I'm curious how this code got broken.

Eric, any ideas? SVN claims that the last change to that line was done
by you (based on a bug *I* reported)? It apparently worked then:

http://sourceforge.net/mailarchive/message.php?msg_name=487A5AE3.5070500%40gmail.com

Ryan

···

On Fri, Aug 27, 2010 at 12:20 PM, Thomas Robitaille <thomas.robitaille@...149...> wrote:

Hi,

The following code:

from matplotlib.patches import Ellipse
from matplotlib.collections import PatchCollection
p = PatchCollection([Ellipse((0.5,0.5),0.2,0.1)], match_original=True)

raises the following exception:

Traceback (most recent call last):
File "test_patches.py", line 5, in <module>
p = PatchCollection([Ellipse((0.5,0.5),0.2,0.1)], match_original=True)
File "/Users/tom/Library/Python/2.6/site-packages/matplotlib/collections.py", line 1041, in __init__
facecolors = [determine_facecolor(p) for p in patches]
File "/Users/tom/Library/Python/2.6/site-packages/matplotlib/collections.py", line 1037, in determine_facecolor
if patch.fill:
AttributeError: 'Ellipse' object has no attribute 'fill'

I have submitted a ticket: https://sourceforge.net/tracker/?group_id=80706&atid=560720

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

#3

Hi,

The following code:

from matplotlib.patches import Ellipse
from matplotlib.collections import PatchCollection
p = PatchCollection([Ellipse((0.5,0.5),0.2,0.1)], match_original=True)

raises the following exception:

Traceback (most recent call last):
  File "test_patches.py", line 5, in<module>
    p = PatchCollection([Ellipse((0.5,0.5),0.2,0.1)], match_original=True)
  File "/Users/tom/Library/Python/2.6/site-packages/matplotlib/collections.py", line 1041, in __init__
    facecolors = [determine_facecolor(p) for p in patches]
  File "/Users/tom/Library/Python/2.6/site-packages/matplotlib/collections.py", line 1037, in determine_facecolor
    if patch.fill:
AttributeError: 'Ellipse' object has no attribute 'fill'

I have submitted a ticket: https://sourceforge.net/tracker/?group_id=80706&atid=560720

The obvious fix would be to change from patch.fill to
patch.get_fill(). However, I'm curious how this code got broken.

I broke it here: http://currents.soest.hawaii.edu/hgstage/hgwebdir.cgi/mpl_hg/diff/799df584a8df/matplotlib/lib/matplotlib/patches.py

The safest way to fix it--that is, preserving the old behavior of patch.fill--would be to make it a property. I can do that later today or tomorrow, or you can go ahead with it. It would be something of an inconsistency, in that for partly historical reasons mpl is based on an endless procession of getters and setters, but I don't think it would do any harm, and it does preserve the earlier API. At the same time, it would be OK to use get_fill in the PatchCollection--it will work, and it is more consistent with typical mpl usage. So, I think the best fix is to make both changes, with a comment in patches.py as to why fill is a property.

Eric

···

On 08/27/2010 09:09 AM, Ryan May wrote:

On Fri, Aug 27, 2010 at 12:20 PM, Thomas Robitaille > <thomas.robitaille@...149...> wrote:

Eric, any ideas? SVN claims that the last change to that line was done
by you (based on a bug *I* reported)? It apparently worked then:

http://sourceforge.net/mailarchive/message.php?msg_name=487A5AE3.5070500%40gmail.com

Ryan

#4

I have submitted a ticket: https://sourceforge.net/tracker/?group_id=80706&atid=560720

The obvious fix would be to change from patch.fill to
patch.get_fill(). However, I'm curious how this code got broken.

I broke it here: http://currents.soest.hawaii.edu/hgstage/hgwebdir.cgi/mpl_hg/diff/799df584a8df/matplotlib/lib/matplotlib/patches.py

The safest way to fix it--that is, preserving the old behavior of patch.fill--would be to make it a property. I can do that later today or tomorrow, or you can go ahead with it. It would be something of an inconsistency, in that for partly historical reasons mpl is based on an endless procession of getters and setters, but I don't think it would do any harm, and it does preserve the earlier API. At the same time, it would be OK to use get_fill in the PatchCollection--it will work, and it is more consistent with typical mpl usage. So, I think the best fix is to make both changes, with a comment in patches.py as to why fill is a property.

I was wondering whether it would be possible at least for now to implement the get_fill() fix in PatchCollection? One of the packages I develop (http://aplpy.sourceforge.net/) depends on match_original, and recent svn versions of matplotlib are unusable because of this.

Thanks!

Tom

···

Eric

Eric, any ideas? SVN claims that the last change to that line was done
by you (based on a bug *I* reported)? It apparently worked then:

http://sourceforge.net/mailarchive/message.php?msg_name=487A5AE3.5070500%40gmail.com

Ryan

#5

It's in my queue to get it done, but probably not until tomorrow.

Ryan

···

On Wed, Sep 1, 2010 at 12:57 PM, Thomas Robitaille <thomas.robitaille@...149...> wrote:

I have submitted a ticket: https://sourceforge.net/tracker/?group_id=80706&atid=560720

The obvious fix would be to change from patch.fill to
patch.get_fill(). However, I'm curious how this code got broken.

I broke it here: http://currents.soest.hawaii.edu/hgstage/hgwebdir.cgi/mpl_hg/diff/799df584a8df/matplotlib/lib/matplotlib/patches.py

The safest way to fix it--that is, preserving the old behavior of patch.fill--would be to make it a property. I can do that later today or tomorrow, or you can go ahead with it. It would be something of an inconsistency, in that for partly historical reasons mpl is based on an endless procession of getters and setters, but I don't think it would do any harm, and it does preserve the earlier API. At the same time, it would be OK to use get_fill in the PatchCollection--it will work, and it is more consistent with typical mpl usage. So, I think the best fix is to make both changes, with a comment in patches.py as to why fill is a property.

I was wondering whether it would be possible at least for now to implement the get_fill() fix in PatchCollection? One of the packages I develop (http://aplpy.sourceforge.net/) depends on match_original, and recent svn versions of matplotlib are unusable because of this.

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

#6

I broke it, so I fixed it: maintenance branch 8673, trunk 8674.

Sorry for the delay.

Eric

···

On 09/01/2010 07:57 AM, Thomas Robitaille wrote:

I have submitted a ticket: https://sourceforge.net/tracker/?group_id=80706&atid=560720

The obvious fix would be to change from patch.fill to
patch.get_fill(). However, I'm curious how this code got broken.

I broke it here: http://currents.soest.hawaii.edu/hgstage/hgwebdir.cgi/mpl_hg/diff/799df584a8df/matplotlib/lib/matplotlib/patches.py

The safest way to fix it--that is, preserving the old behavior of patch.fill--would be to make it a property. I can do that later today or tomorrow, or you can go ahead with it. It would be something of an inconsistency, in that for partly historical reasons mpl is based on an endless procession of getters and setters, but I don't think it would do any harm, and it does preserve the earlier API. At the same time, it would be OK to use get_fill in the PatchCollection--it will work, and it is more consistent with typical mpl usage. So, I think the best fix is to make both changes, with a comment in patches.py as to why fill is a property.

I was wondering whether it would be possible at least for now to implement the get_fill() fix in PatchCollection? One of the packages I develop (http://aplpy.sourceforge.net/) depends on match_original, and recent svn versions of matplotlib are unusable because of this.