Hi, I was shown a bug today by a colleague, demonstrated by the following example: import numpy as np import matplotlib.pyplot as plt a = np.zeros([10, 10]) a[2:6,3:8] = 1 ls = plt.contour(a, 1, colors='r', linewidths=3, zorder=5) print ls.collections[0]l.get_zorder() rect = plt.Rectangle([2, 3], 5, 4, zorder=3) plt.gca().add_artist(rect) plt.show() Unless I completely misunderstand zorder, the contour should be *on top* of the rectangle. Also note that printing the zorder for the contour's collection (a LineCollection object) gives a value of 2, even though the call to contour() specified 5 for the zorder. Looking over the code for contour, the zorder is never mentioned. The reason the LineCollection ends up with a value of 2 is that this is the default for a LineCollection. My question is: is there any reason *not* to use the passed in zorder for contours/filled contours? If this is the proper fix, I'll check it in myself, I just wanted to make sure I'm not missing some special case here. As an aside, this is yet another example where it would be nice to know that a keyword argument was not being used. If there's no objections, I'd be willing to change ContourSet to pop arguments off of **kwargs so that it can see if some aren't used and throw an exception if not all are used. On the other hand, this could break existing code that are passing extra/useless kwargs, so maybe a warning would be better? Ryan -- Ryan May Graduate Research Assistant School of Meteorology University of Oklahoma
Ryan May wrote: > Hi, > > I was shown a bug today by a colleague, demonstrated by the following example: > > import numpy as np > import matplotlib.pyplot as plt > > a = np.zeros([10, 10]) > a[2:6,3:8] = 1 > ls = plt.contour(a, 1, colors='r', linewidths=3, zorder=5) > print ls.collections[0]l.get_zorder() > rect = plt.Rectangle([2, 3], 5, 4, zorder=3) > plt.gca().add_artist(rect) > plt.show() > > Unless I completely misunderstand zorder, the contour should be *on > top* of the rectangle. Also note that printing the zorder for the > contour's collection (a LineCollection object) gives a value of 2, > even though the call to contour() specified 5 for the zorder. Looking > over the code for contour, the zorder is never mentioned. The reason > the LineCollection ends up with a value of 2 is that this is the > default for a LineCollection. My question is: is there any reason > *not* to use the passed in zorder for contours/filled contours? If > this is the proper fix, I'll check it in myself, I just wanted to make > sure I'm not missing some special case here. Ryan, Certainly it makes sense to support zorder in some fashion, and the simple way is as you suggest, with one value per call to contour. It may be best to stop there--but I can imagine the next complaint being, "why doesn't zorder support a sequence?", and then things get quite a bit more complicated. Anyway, go ahead and put in the simple zorder support; I don't see any downside to it. > > As an aside, this is yet another example where it would be nice to > know that a keyword argument was not being used. If there's no > objections, I'd be willing to change ContourSet to pop arguments off > of **kwargs so that it can see if some aren't used and throw an > exception if not all are used. On the other hand, this could break > existing code that are passing extra/useless kwargs, so maybe a > warning would be better? Careful. Keeping track of which kwargs are used, and making sure they are always popped, and don't need to remain in the kwargs dictionary, could get tricky. I just added support for the units-related kwargs. I think what you suggest will work, and I agree that some error-trapping for kwarg accidents (misspelling kwargs, or otherwise trying to use a kwarg that has no effect) is a good idea. This sounds like an area where some general mpl coding guidelines, and quite a bit of work to implement them, would be good. Eric > > Ryan >
On Thu, Jan 28, 2010 at 1:03 PM, Eric Firing <ef...@ha...> wrote: > Ryan May wrote: >> Unless I completely misunderstand zorder, the contour should be *on >> top* of the rectangle. Also note that printing the zorder for the >> contour's collection (a LineCollection object) gives a value of 2, >> even though the call to contour() specified 5 for the zorder. Looking >> over the code for contour, the zorder is never mentioned. The reason >> the LineCollection ends up with a value of 2 is that this is the >> default for a LineCollection. My question is: is there any reason >> *not* to use the passed in zorder for contours/filled contours? If >> this is the proper fix, I'll check it in myself, I just wanted to make >> sure I'm not missing some special case here. > > Ryan, > > Certainly it makes sense to support zorder in some fashion, and the simple > way is as you suggest, with one value per call to contour. It may be best > to stop there--but I can imagine the next complaint being, "why doesn't > zorder support a sequence?", and then things get quite a bit more > complicated. > > Anyway, go ahead and put in the simple zorder support; I don't see any > downside to it. Will do shortly. >> As an aside, this is yet another example where it would be nice to >> know that a keyword argument was not being used. If there's no >> objections, I'd be willing to change ContourSet to pop arguments off >> of **kwargs so that it can see if some aren't used and throw an >> exception if not all are used. On the other hand, this could break >> existing code that are passing extra/useless kwargs, so maybe a >> warning would be better? > > Careful. Keeping track of which kwargs are used, and making sure they are > always popped, and don't need to remain in the kwargs dictionary, could get > tricky. I just added support for the units-related kwargs. I think what > you suggest will work, and I agree that some error-trapping for kwarg > accidents (misspelling kwargs, or otherwise trying to use a kwarg that has > no effect) is a good idea. > > This sounds like an area where some general mpl coding guidelines, and quite > a bit of work to implement them, would be good. Agreed. After I wrote this, I thought about it some more, and it's not something you can really do piecemeal due to passing off to base classes and what not. It's a big usability wart however, like when accidentally using 'linewidth' instead of 'linewidths' for calling contour. I'll just leave this as a future improvement. Ryan -- Ryan May Graduate Research Assistant School of Meteorology University of Oklahoma Sent from Norman, Oklahoma, United States