Thanks everyone for the feedback. I have made Spine subclass Patch in svn r7170, which I just committed. This resulted in a slight API change, but addresses most of the more substantial points raised.
The slight API change is that spine.set_color() is now spine.set_edgecolor(). More below. > On Thu, May 28, 2009 at 9:18 PM, John Hunter <jdh2...@gmail.com> wrote: >> You do an isinstance(arg, basestring) to check for string input. >> Typically, we encourage cbook.is_string_like to have a central point >> of maintenance and consistency for these checks. fixed in r7169 >> Also, in the example, you appear to turn off a spine by setting the >> color to 'none'. My thought it would be more natural to use the >> "visible" artist property here (or at least support both) I think this is addressed naturally by the new "Spine is a Patch" behavior. >> Also, I think the class of strings representing "no color" in mpl is >> larger -- it should also include self.color.lower()=='none' and the >> empty string, which I've included in the example code. Same for this. Now there's a single point of failure in the Patch.draw() method. Jae-Joon Lee wrote: > The zorder of the spine artists is set to 0 by default. > I notice that you're changing the zorder of its artist attribute, but > note that it has no effect as what matter is the zorder of the spine > itself. Again, I think this is dealt with by the "Spine is a Patch" patch. > As a related issue to what John mentioned, I think one option you can > do is to derive the Spine class itself from a real artist class, > rather than the base "Artist". With this, you don't need to implement > all other set/get method, e.g., color, etc. For example, you may > derive it from the Patch class. Note that while the Patch class is > intended for closed path, you can stroke a straight line with it. Good idea -- done! :) > * cla() does not reset spines (positions, color, etc). I think it is > better to be reset, since all other things are. For example, cla() > resets visibility of ticks, etc. Nice catch. Fixed in r7168. > * better support for log scale. I see the issue here, but I haven't had a chance to fix it. To be honest, I'm surprised there aren't more of these types of issues... You're welcome to take a look if you're so inclined -- it'll probably be a few days before I have a chance to look at it. ------------------------------------------------------------------------------ OpenSolaris 2009.06 is a cutting edge operating system for enterprises looking to deploy the next generation of Solaris that includes the latest innovations from Sun and the OpenSource community. Download a copy and enjoy capabilities such as Networking, Storage and Virtualization. Go to: http://p.sf.net/sfu/opensolaris-get _______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
