I've gone ahead and committed my arbitrary spine location implementation
to the trunk (svn r7144). I'd appreciate it if you could kick the tires.
To get you started, try the new demo:
examples/pylab_examples/spine_placement_demo.py
I believe I addressed all the issues raised with the patch I emailed the
list last week and I tried to avoid any breakage. Thanks to all who
commented -- you made this a better implementation.
Note that Axes.frame no longer exists, and I made a note of this in
api_changes.rst and a hopefully carefully worded AttributeError will be
raised if you try to access it.
Also, as excercised by the demo, in addition to support for a offset of
spines specified in points, one may specify spine placement in both axes
and data coordinates. Here is the docstring for Spine.set_position:
"""
set the position of the spine
Spine position is specified by a 2 tuple of (position type,
amount). The position types are:
* 'outward' : place the spine out from the data area by the
specified number of points. (Negative values specify placing the
spine inward.)
* 'axes' : place the spine at the specified Axes coordinate (from
0.0-1.0).
* 'data' : place the spine at the specified data coordinate.
Additionally, shorthand notations define a special positions:
* 'center' -> ('axes',0.5)
* 'zero' -> ('data', 0.0)
"""
As always, please let me know of any suggestions or comments.
-Andrew
Andrew Straw wrote: > I believe I addressed all the issues raised with the patch I emailed the > list last week and I tried to avoid any breakage. Thanks to all who > commented -- you made this a better implementation. Upon further reflection, I realize I didn't add any Axes convenience methods as Eric suggested. This is simply due to a lack of time -- not a lack of enthusiasm. -Andrew
Andrew Straw wrote: > Andrew Straw wrote: >> I believe I addressed all the issues raised with the patch I emailed the >> list last week and I tried to avoid any breakage. Thanks to all who >> commented -- you made this a better implementation. > > Upon further reflection, I realize I didn't add any Axes convenience > methods as Eric suggested. This is simply due to a lack of time -- not a > lack of enthusiasm. Andrew, No problem--it can be done later, no rush. Your replacement of the frame with spines is a *big* improvement. Thank you for the great work. Eric
On Wed, May 27, 2009 at 12:53 PM, Eric Firing <ef...@ha...> wrote: > No problem--it can be done later, no rush. Your replacement of the > frame with spines is a *big* improvement. Thank you for the great work. Yes, this looks great -- thanks again Andrew. Something that would be nice would be a Formatter that is aware of the spines which would drop the label in the places near the spine intersection (eg the zeros in the centered example). Nothing obvious comes to mind, but I just wanted to throw it out there in case someone wants to dig in. JDH
Thanks a lot Andrew. This looks great.
I'm just reporting some of issues I encountered in a hope that you can
address these (I'll also take a look if have chance).
* 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.
* better support for log scale.
ax = subplot(131)
ax.spines["bottom"].set_position("center")
ax.semilogx()
# this works
ax = subplot(132)
ax.semilogx()
ax.spines["bottom"].set_position("center")
# this does NOT
ax = subplot(133)
ax.spines["bottom"].set_position(("data", 1))
ax.loglog()
# this does NOT work regardless the position of loglog.
▶◀
-JJ
On Wed, May 27, 2009 at 12:33 PM, Andrew Straw <str...@as...> wrote:
> I've gone ahead and committed my arbitrary spine location implementation
> to the trunk (svn r7144). I'd appreciate it if you could kick the tires.
> To get you started, try the new demo:
> examples/pylab_examples/spine_placement_demo.py
>
> I believe I addressed all the issues raised with the patch I emailed the
> list last week and I tried to avoid any breakage. Thanks to all who
> commented -- you made this a better implementation.
>
> Note that Axes.frame no longer exists, and I made a note of this in
> api_changes.rst and a hopefully carefully worded AttributeError will be
> raised if you try to access it.
>
> Also, as excercised by the demo, in addition to support for a offset of
> spines specified in points, one may specify spine placement in both axes
> and data coordinates. Here is the docstring for Spine.set_position:
>
> """
> set the position of the spine
>
> Spine position is specified by a 2 tuple of (position type,
> amount). The position types are:
>
> * 'outward' : place the spine out from the data area by the
> specified number of points. (Negative values specify placing the
> spine inward.)
>
> * 'axes' : place the spine at the specified Axes coordinate (from
> 0.0-1.0).
>
> * 'data' : place the spine at the specified data coordinate.
>
> Additionally, shorthand notations define a special positions:
>
> * 'center' -> ('axes',0.5)
> * 'zero' -> ('data', 0.0)
> """
>
> As always, please let me know of any suggestions or comments.
>
> -Andrew
>
> ------------------------------------------------------------------------------
> Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
> is a gathering of tech-side developers & brand creativity professionals. Meet
> the minds behind Google Creative Lab, Visual Complexity, Processing, &
> iPhoneDevCamp as they present alongside digital heavyweights like Barbarian
> Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com
> _______________________________________________
> Matplotlib-devel mailing list
> Mat...@li...
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
>
On Wed, May 27, 2009 at 11:33 AM, Andrew Straw <str...@as...> wrote: > I've gone ahead and committed my arbitrary spine location implementation > to the trunk (svn r7144). I'd appreciate it if you could kick the tires. > To get you started, try the new demo: > examples/pylab_examples/spine_placement_demo.py I just did a quick read through of the spine code and example, and have two minor comments. 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. 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) @allow_rasterization def draw(self,renderer): "draw everything that belongs to the spine" if not self.get_visible() or self.color.lower()=='none' or not self.color: # don't draw invisible spines return self.artist.draw(renderer) 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. JDH
Andrew, Another issue. 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. 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. Regards, -JJ On Thu, May 28, 2009 at 9:18 PM, John Hunter <jd...@gm...> wrote: > On Wed, May 27, 2009 at 11:33 AM, Andrew Straw <str...@as...> wrote: >> I've gone ahead and committed my arbitrary spine location implementation >> to the trunk (svn r7144). I'd appreciate it if you could kick the tires. >> To get you started, try the new demo: >> examples/pylab_examples/spine_placement_demo.py > > I just did a quick read through of the spine code and example, and > have two minor comments. > > 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. > > 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) > > @allow_rasterization > def draw(self,renderer): > "draw everything that belongs to the spine" > if not self.get_visible() or self.color.lower()=='none' or not > self.color: > # don't draw invisible spines > return > self.artist.draw(renderer) > > 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. > > JDH > > ------------------------------------------------------------------------------ > Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT > is a gathering of tech-side developers & brand creativity professionals. Meet > the minds behind Google Creative Lab, Visual Complexity, Processing, & > iPhoneDevCamp as they present alongside digital heavyweights like Barbarian > Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com > _______________________________________________ > Matplotlib-devel mailing list > Mat...@li... > https://lists.sourceforge.net/lists/listinfo/matplotlib-devel >
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 <jd...@gm...> 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.
Andrew Straw wrote: > 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(). But spine.set_color() is much more natural and easy to remember, so maybe it can be restored: Collections have had a set_color() for a long time, and I don't see any reason Patch shouldn't have the same, so I added it. My first thought was that this would have no effect on spines except to permit the alternative and more natural "spine.set_color()" to work like set_edgecolor, but now I see that this is not true--in the case of a circular spine, calling set_color(c) would have the unintended effect of filling the circle. I still think having the set_color method in Patch and Spine is good, so what I propose is that Spine override the Patch version with a simple alias to set_edgecolor. Eric
Eric Firing wrote: > Andrew Straw wrote: >> The slight API change is that spine.set_color() is now >> spine.set_edgecolor(). > > But spine.set_color() is much more natural and easy to remember, so > maybe it can be restored: Good idea. I just re-added Spine.set_color() and changed the example back. -Andrew