On Sun, Nov 30, 2008 at 11:53 AM, Jae-Joon Lee <[EMAIL PROTECTED]> wrote:

> I have been working on reimplementation of the legend class, and I
> guess it is mostly done. The biggest change is how each items are
> placed. For this, I made a simple container class("offsetbox.py"),
> which can pack the mpl artists in a similar way as widget packing in
> gtk, tk, etc.

Hi Jae-Joon -- as usual this is great work, and brings one of the
oldest, hairiest, parts of the code into the modern age.  I went ahead
and committed this to svn r6461 with some minor spelling corrections,
but I have some minor comments below which may be worth addressing in
a revision.

Aside from the legend enhancement, the important and general piece
here is the offset box, which  could become a significant piece of the
low-level infrastructure (eg for axes layout)  so I do want to make
sure we get this right.  In particular, I'd like Andrew Straw, who has
a toolkit for sizers, and Michael Droettboom, who implemented the
Knuth boxes in support of mathtext, to take a look at the API and
provide some feedback as to whether if it looks sufficient.

I have some minor comments from the read-thru

+        self.legendPatch = FancyBboxPatch(
+            xy=(0.0, 0.0), width=1., height=1.,
             facecolor='w', edgecolor='k',
+            mutation_scale=self.fontsize,
             )

Should we make this an option that defaults to True, eg fancybox=True,
so those who want an old-style rectangle can still get it?

+        largecol = zip(range(0, num_largecol*(nrows+1), (nrows+1)),
+                       [nrows+1] * num_largecol)

In general, we encourage the use of cbook.safezip over zip because it
traps hard to find bugs.

+class OffsetBox(martist.Artist):
+    """
+    The OffsetBox is a simple container artist. The child artist are meant
+    to be drawn at a relative position to its parent.
+    """
+    def __init__(self, *kl, **kw):

I would like to stick to a standard *args, **kwargs naming convention
in mpl code

+    def set_offset(self, xy):
+        """
+        set offset of the container.
+
+        Accept : tuple of x,y cooridnate in display units.

Is it worthwhile to allow other coordinate systems for the offsets
(points, data) or would this complicate the code with little benefit?
On a quick read-thru, it looks like we would just need to supply an
optional transform arg and transform the offset if necessary, so the
implementation would be pretty clean.  I see that offset can be
callable -- is this the mechanism you intended to handle this case?

+    'legend.borderpad'   : [0.4, validate_float], # units are fontsize

I think you mean the "value is a fraction of the fontsize".  In
general, there are a number of places where it is not clear from the
docstring what the units are (eg in the OffsetBox.set_offset)., so
this could be cleaned up a bit.


+ax1.plot([1], label="multi\nline")

For multi-line|, it appears that we are getting the default "baseline"
vertical alignment but for the legend, 'center' would probably be more
appropriate.  Any idea how to get that to work?

Thanks again!
JDH

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Reply via email to