I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
This change occurred as part of the big push to draw everything as polycurves. If we decide to support implicit closing of polygons, we can probably do that within the polygon patch class.
Cheers,
Mike
Eric Firing wrote:
···
Mike,
Brian F. has correctly pointed out a change in behavior from the maintenance branch to the trunk: in the former, the fill command does not require that the polygon be closed--that is, that the last point coincide with the first. Hence, a filled rectangle can be specified with a list of 4 points. This seems to me like desirable behavior. After a very cursory look I have not spotted any obvious place where this change occurred, but I suspect it will be immediately obvious to you.
Should the old behavior be restored?
Thanks.
Eric
------------------------------------------------------------------------
Subject:
Re: [Matplotlib-users] missing lines on graph with upgrade to 0.98.0
From:
Bryan Fodness <bryan.fodness@...149...>
Date:
Thu, 05 Jun 2008 08:38:59 -0400
To:
Eric Firing <efiring@...229...>
To:
Eric Firing <efiring@...229...>
CC:
Matplotlib <Matplotlib-users@lists.sourceforge.net>
It seems like it does not connect the last point to the first point. This also happens with the matplotlib.patches Polygon.
x1, x2, y1, y2 = -4, 4, -4, 4
fill([x1,x2,x2,x1], [y1,y1,y2,y2], fc='None', ec='r')
xlim(-5,5)
ylim(-5,5)
savefig('edge_test')
On Thu, Jun 5, 2008 at 1:18 AM, Eric Firing <efiring@...229... > <mailto:efiring@…229…>> wrote:
Bryan Fodness wrote:
I just upgraded to 0.98.0 and recreated a few graphs. I am
missing parts of the edges of a fill and polygon. Any
suggestions?
Please post an illustrative script, as simple as possible.
Eric
--
"The game of science can accurately be described as a never-ending insult to human intelligence." - Jo�o Magueijo
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Eric
···
This change occurred as part of the big push to draw everything as polycurves. If we decide to support implicit closing of polygons, we can probably do that within the polygon patch class.
Cheers,
Mike
Eric Firing wrote:
Mike,
Brian F. has correctly pointed out a change in behavior from the maintenance branch to the trunk: in the former, the fill command does not require that the polygon be closed--that is, that the last point coincide with the first. Hence, a filled rectangle can be specified with a list of 4 points. This seems to me like desirable behavior. After a very cursory look I have not spotted any obvious place where this change occurred, but I suspect it will be immediately obvious to you.
Should the old behavior be restored?
Thanks.
Eric
------------------------------------------------------------------------
Subject:
Re: [Matplotlib-users] missing lines on graph with upgrade to 0.98.0
From:
Bryan Fodness <bryan.fodness@...149...>
Date:
Thu, 05 Jun 2008 08:38:59 -0400
To:
Eric Firing <efiring@...229...>
To:
Eric Firing <efiring@...229...>
CC:
Matplotlib <Matplotlib-users@lists.sourceforge.net>
It seems like it does not connect the last point to the first point. This also happens with the matplotlib.patches Polygon.
x1, x2, y1, y2 = -4, 4, -4, 4
fill([x1,x2,x2,x1], [y1,y1,y2,y2], fc='None', ec='r')
xlim(-5,5)
ylim(-5,5)
savefig('edge_test')
On Thu, Jun 5, 2008 at 1:18 AM, Eric Firing <efiring@...229... >> <mailto:efiring@…229…>> wrote:
Bryan Fodness wrote:
I just upgraded to 0.98.0 and recreated a few graphs. I am
missing parts of the edges of a fill and polygon. Any
suggestions?
Please post an illustrative script, as simple as possible.
Eric
--
"The game of science can accurately be described as a never-ending insult to human intelligence." - Jo�o Magueijo
Eric Firing wrote:
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Here is the Matlab help text; matlab does automatically close the polygons:
fill(X,Y,C) creates filled polygons from the data in X and Y with vertex color specified by C. C is a vector or matrix used as an index into the colormap. If C is a row vector, length(C) must equal size(X,2) and size(Y,2); if C is a column vector, length(C) must equal size(X,1) and size(Y,1). If necessary, fill closes the polygon by connecting the last vertex to the first.
Eric
Thanks. That's a good argument to do the close for fill(). I'll wait a bit to see if others chime in, but at least at that level it seems to be a no-brainer. Whether we want to do this in the Polygon patch is still an open question, perhaps.
Cheers,
Mike
Eric Firing wrote:
···
Eric Firing wrote:
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Here is the Matlab help text; matlab does automatically close the polygons:
fill(X,Y,C) creates filled polygons from the data in X and Y with vertex color specified by C. C is a vector or matrix used as an index into the colormap. If C is a row vector, length(C) must equal size(X,2) and size(Y,2); if C is a column vector, length(C) must equal size(X,1) and size(Y,1). If necessary, fill closes the polygon by connecting the last vertex to the first.
Eric
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA
Michael Droettboom wrote:
Thanks. That's a good argument to do the close for fill(). I'll wait a bit to see if others chime in, but at least at that level it seems to be a no-brainer. Whether we want to do this in the Polygon patch is still an open question, perhaps.
Mike,
Let's see if anyone says anything either way. If no one does, then I suggest that you should be the one to decide whether it makes sense to make the change in patches or in fill. If the ultimate decision is to change patches, then that is simpler, and there is no point in making the slightly more complicated changes in axes. In either case, I think the closing should be done only if a test shows that the points passed in are not already closed.
Looking at patches a little more, I think I would be inclined to put the change in Polygon, on the grounds that a polygon simply is a *closed* path specified by its vertices; there should be no need to explicitly close it, although it may be more efficient to do so. For the case where someone wants a general path, it looks like you have thoughtfully provided the PathPatch object, so we don't really lose generality by forcing the Polygon to be closed.
Eric
···
Cheers,
Mike
Eric Firing wrote:
Eric Firing wrote:
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Here is the Matlab help text; matlab does automatically close the polygons:
fill(X,Y,C) creates filled polygons from the data in X and Y with vertex color specified by C. C is a vector or matrix used as an index into the colormap. If C is a row vector, length(C) must equal size(X,2) and size(Y,2); if C is a column vector, length(C) must equal size(X,1) and size(Y,1). If necessary, fill closes the polygon by connecting the last vertex to the first.
Eric
I've gone ahead and fixed this in the Polygon patch. As you point out, if someone wants an open polygon, they can use PathPatch, and Polygon was never able to do that before anyway.
Cheers,
Mike
Eric Firing wrote:
···
Michael Droettboom wrote:
Thanks. That's a good argument to do the close for fill(). I'll wait a bit to see if others chime in, but at least at that level it seems to be a no-brainer. Whether we want to do this in the Polygon patch is still an open question, perhaps.
Mike,
Let's see if anyone says anything either way. If no one does, then I suggest that you should be the one to decide whether it makes sense to make the change in patches or in fill. If the ultimate decision is to change patches, then that is simpler, and there is no point in making the slightly more complicated changes in axes. In either case, I think the closing should be done only if a test shows that the points passed in are not already closed.
Looking at patches a little more, I think I would be inclined to put the change in Polygon, on the grounds that a polygon simply is a *closed* path specified by its vertices; there should be no need to explicitly close it, although it may be more efficient to do so. For the case where someone wants a general path, it looks like you have thoughtfully provided the PathPatch object, so we don't really lose generality by forcing the Polygon to be closed.
Eric
Cheers,
Mike
Eric Firing wrote:
Eric Firing wrote:
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Here is the Matlab help text; matlab does automatically close the polygons:
fill(X,Y,C) creates filled polygons from the data in X and Y with vertex color specified by C. C is a vector or matrix used as an index into the colormap. If C is a row vector, length(C) must equal size(X,2) and size(Y,2); if C is a column vector, length(C) must equal size(X,1) and size(Y,1). If necessary, fill closes the polygon by connecting the last vertex to the first.
Eric
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA
Michael Droettboom wrote:
I've gone ahead and fixed this in the Polygon patch. As you point out, if someone wants an open polygon, they can use PathPatch, and Polygon was never able to do that before anyway.
Cheers,
Mike
Hi Mike,
1) I think that there is a bug in the patch: xy is a (N,2) array and I get an error message for the line
if len(xy) and xy[0] !=xy[-1]:
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
so, I think it should really be
if len(xy) and (xy[0] !=xy[-1]).any():
2) With this change the output hist doesn't look that good any more, since it relies on the axes fill method. axes fill produced a filled patch where the enclosing line was not closed (i.e. open at bottom of the hist). So you could easily produce a nice unfilled step line-histogram and I intended to make this the default behaviour for hist(histtype='step'). Now the line gets closed which most users - I guess - don't want to.
So maybe it is needed to have a keyword for the axes fill method to control whether the patch should be closed or not.
Manuel
···
Eric Firing wrote:
Michael Droettboom wrote:
Thanks. That's a good argument to do the close for fill(). I'll wait a bit to see if others chime in, but at least at that level it seems to be a no-brainer. Whether we want to do this in the Polygon patch is still an open question, perhaps.
Mike,
Let's see if anyone says anything either way. If no one does, then I suggest that you should be the one to decide whether it makes sense to make the change in patches or in fill. If the ultimate decision is to change patches, then that is simpler, and there is no point in making the slightly more complicated changes in axes. In either case, I think the closing should be done only if a test shows that the points passed in are not already closed.
Looking at patches a little more, I think I would be inclined to put the change in Polygon, on the grounds that a polygon simply is a *closed* path specified by its vertices; there should be no need to explicitly close it, although it may be more efficient to do so. For the case where someone wants a general path, it looks like you have thoughtfully provided the PathPatch object, so we don't really lose generality by forcing the Polygon to be closed.
Eric
Cheers,
Mike
Eric Firing wrote:
Eric Firing wrote:
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Here is the Matlab help text; matlab does automatically close the polygons:
fill(X,Y,C) creates filled polygons from the data in X and Y with vertex color specified by C. C is a vector or matrix used as an index into the colormap. If C is a row vector, length(C) must equal size(X,2) and size(Y,2); if C is a column vector, length(C) must equal size(X,1) and size(Y,1). If necessary, fill closes the polygon by connecting the last vertex to the first.
Eric
--
---------------------------------------
Manuel Metz ............ Stw@...468...
Argelander Institut fuer Astronomie
Auf dem Huegel 71 (room 3.06)
D - 53121 Bonn
E-Mail: mmetz@...459...
Web: www.astro.uni-bonn.de/~mmetz
Phone: (+49) 228 / 73-3660
Fax: (+49) 228 / 73-3672
---------------------------------------
Thanks. I have fixed 1), and added a "closed" kwarg to fill() and Polygon.__init__() (which defaults to True to mimic behavior of 0.91 and earlier). hist() has been updated to call fill() with closed=False.
Cheers,
Mike
Manuel Metz wrote:
···
Michael Droettboom wrote:
I've gone ahead and fixed this in the Polygon patch. As you point out, if someone wants an open polygon, they can use PathPatch, and Polygon was never able to do that before anyway.
Cheers,
Mike
Hi Mike,
1) I think that there is a bug in the patch: xy is a (N,2) array and I get an error message for the line
if len(xy) and xy[0] !=xy[-1]:
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
so, I think it should really be
if len(xy) and (xy[0] !=xy[-1]).any():
2) With this change the output hist doesn't look that good any more, since it relies on the axes fill method. axes fill produced a filled patch where the enclosing line was not closed (i.e. open at bottom of the hist). So you could easily produce a nice unfilled step line-histogram and I intended to make this the default behaviour for hist(histtype='step'). Now the line gets closed which most users - I guess - don't want to.
So maybe it is needed to have a keyword for the axes fill method to control whether the patch should be closed or not.
Manuel
Eric Firing wrote:
Michael Droettboom wrote:
Thanks. That's a good argument to do the close for fill(). I'll wait a bit to see if others chime in, but at least at that level it seems to be a no-brainer. Whether we want to do this in the Polygon patch is still an open question, perhaps.
Mike,
Let's see if anyone says anything either way. If no one does, then I suggest that you should be the one to decide whether it makes sense to make the change in patches or in fill. If the ultimate decision is to change patches, then that is simpler, and there is no point in making the slightly more complicated changes in axes. In either case, I think the closing should be done only if a test shows that the points passed in are not already closed.
Looking at patches a little more, I think I would be inclined to put the change in Polygon, on the grounds that a polygon simply is a *closed* path specified by its vertices; there should be no need to explicitly close it, although it may be more efficient to do so. For the case where someone wants a general path, it looks like you have thoughtfully provided the PathPatch object, so we don't really lose generality by forcing the Polygon to be closed.
Eric
Cheers,
Mike
Eric Firing wrote:
Eric Firing wrote:
Michael Droettboom wrote:
I'm not entirely certain this is desirable behavior -- what if the user *wants* to draw an open-yet-filled polygon? How could that be done? (Admittedly, it couldn't be done before). It seems more general to require the user to close polygons.
True. I don't feel strongly about this. My guess is that at least at the level of the Axes.fill method, a user would almost never want the open-yet-filled case, but I could be guessing wrong, or the "almost" qualifier could be critical. We could do automatic closing only at that level, however.
Maybe the best alternative is to leave the trunk behavior as it is, and make sure the documentation is very explicit about the need to supply a closed path. This change could be added to API_CHANGES, as well as to the Axes.fill docstring.
Does anyone know how Matlab, IDL, etc. handle this?
Here is the Matlab help text; matlab does automatically close the polygons:
fill(X,Y,C) creates filled polygons from the data in X and Y with vertex color specified by C. C is a vector or matrix used as an index into the colormap. If C is a row vector, length(C) must equal size(X,2) and size(Y,2); if C is a column vector, length(C) must equal size(X,1) and size(Y,1). If necessary, fill closes the polygon by connecting the last vertex to the first.
Eric
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA
Michael Droettboom wrote:
Thanks. I have fixed 1), and added a "closed" kwarg to fill() and Polygon.__init__() (which defaults to True to mimic behavior of 0.91 and earlier). hist() has been updated to call fill() with closed=False.
Cheers,
Mike
Great - thanks. So, I committed a patch with a further update of hist() introducing some new nice features.
Cheers,
Manuel