Hello, 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. * paddings are given as fraction of the font size. * baseline alignment (when supported by backend) * multicolumn * expand-mode (see the last panel in the attached figure) * fancybox for the frame It should be backward compatible. The backend_driver.py runs fine and all the examples I checked looks good. So, here is the patch and example figure. It would be great if others review and test this patch. Regards, -JJ
On Sun, Nov 30, 2008 at 11:53 AM, Jae-Joon Lee <lee...@gm...> 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
John Hunter wrote: > On Sun, Nov 30, 2008 at 11:53 AM, Jae-Joon Lee <lee...@gm...> 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. > Hi Jae-Joon and John, The MPL sizer toolkits looks entirely unaffected by these changes, and the demo looks very convincing. So from my perspective this would be a benefit to have in assuming that the issues John raises can be dealt with.
Thanks John, I'll work on the revisions. > > + 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? > My original intention with the OffsetBox was to only support the translation in the display coordinates. VPacker and HPacker even doesn't use the transformation explicitly. One of the reason for this was that the size of the Text objects does not depends on the transformation. I guess in most cases, children will have a fixed offset relative to their parent container. Introducing an arbitrary transformation between the child and the parent may make things complicated. On the other hand, the top most container may have its own transformation. In the use case I considered (legend), the size of the box is only known at the drawing time and the offset of the box needs to be calculated on the fly (e.g., using the size of the box and the anchor point). The reason that the offset can be a callable object is to support such situation. As implementing an optional transformation should be trivial, I'll just go ahead an work on it. But it seems not clear to me how to approach this without any specific use case given. > > +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? > We may do it by adjusting the baseline of the multi-line text. In the current implementation, the baseline of the multi-line text is set to the baseline of the first line. It may not give the perfect center-align, but will give a reasonable results. The other way is to treat the multiline text separately during the packing. But this will need more work as the current Packer class does not know whether its child is multi-line text or not. I'll see what I can do. Regards, -JJ
On Mon, Dec 1, 2008 at 6:00 PM, Jae-Joon Lee <lee...@gm...> wrote: > As implementing an optional transformation should be trivial, I'll > just go ahead an work on it. But it seems not clear to me how to > approach this without any specific use case given. In another thread yesterday, someone was asking about a general way to create a shadow effect. It occurred to me that we could probably use the offsetbox for this. One could create an offset box for a given artist that draws the artist offset from the original by 5 points or 5 pixels, in a gray color that creates the appearance of a shadow. Would this be a reasonable use case to test the offset transformation in either points or pixels? JDH
I think something broke with the recent changes to legend. For example, in ipython -pylab: plot([1,2,3,4],label='test') legend(loc=(.1, .5)) ERROR: An unexpected error occurred while tokenizing input The following traceback may be corrupted or invalid The error message is: ('EOF in multi-line statement', (185, 0)) ERROR: An unexpected error occurred while tokenizing input The following traceback may be corrupted or invalid The error message is: ('EOF in multi-line statement', (46, 0)) --------------------------------------------------------------------------- AssertionError Traceback (most recent call last) /home/darren/<ipython console> in <module>() /usr/lib64/python2.6/site-packages/matplotlib/pyplot.pyc in legend(*args, **kwargs) 2441 2442 ret = gca().legend(*args, **kwargs) -> 2443 draw_if_interactive() 2444 return ret 2445 if Axes.legend.__doc__ is not None: /usr/lib64/python2.6/site-packages/matplotlib/backends/backend_qt4.pyc in draw_if_interactive() 38 figManager = Gcf.get_active() 39 if figManager != None: ---> 40 figManager.canvas.draw() 41 42 def _create_qApp(): /usr/lib64/python2.6/site-packages/matplotlib/backends/backend_qt4agg.pyc in draw(self) 131 if DEBUG: print "FigureCanvasQtAgg.draw", self 132 self.replot = True --> 133 FigureCanvasAgg.draw(self) 134 self.update() 135 # Added following line to improve realtime pan/zoom on windows: /usr/lib64/python2.6/site-packages/matplotlib/backends/backend_agg.pyc in draw(self) 281 282 self.renderer = self.get_renderer() --> 283 self.figure.draw(self.renderer) 284 285 def get_renderer(self): /usr/lib64/python2.6/site-packages/matplotlib/figure.pyc in draw(self, renderer) 770 771 # render the axes --> 772 for a in self.axes: a.draw(renderer) 773 774 # render the figure text /usr/lib64/python2.6/site-packages/matplotlib/axes.pyc in draw(self, renderer, inframe) 1599 1600 for zorder, i, a in dsu: -> 1601 a.draw(renderer) 1602 1603 renderer.close_group('axes') /usr/lib64/python2.6/site-packages/matplotlib/legend.pyc in draw(self, renderer) 317 if self._drawFrame: 318 # update the location and size of the legend --> 319 bbox = self._legend_box.get_window_extent(renderer) 320 self.legendPatch.set_bounds(bbox.x0, bbox.y0, 321 bbox.width, bbox.height) /usr/lib64/python2.6/site-packages/matplotlib/offsetbox.pyc in get_window_extent(self, renderer) 196 ''' 197 w, h, xd, yd, offsets = self.get_extent_offsets(renderer) --> 198 px, py = self.get_offset(w, h, xd, yd) 199 return mtransforms.Bbox.from_bounds(px-xd, py-yd, w, h) 200 /usr/lib64/python2.6/site-packages/matplotlib/offsetbox.pyc in get_offset(self,width, height, xdescent, ydescent) 155 """ 156 if callable(self._offset): --> 157 return self._offset(width, height, xdescent, ydescent) 158 else: 159 return self._offset /usr/lib64/python2.6/site-packages/matplotlib/legend.pyc in _findoffset_loc(self, width, height, xdescent, ydescent) 292 "Heper function to locate the legend" 293 bbox = Bbox.from_bounds(0, 0, width, height) --> 294 x, y = self._get_anchored_bbox(self._loc, bbox, self.parent.bbox) 295 return x+xdescent, y+ydescent 296 /usr/lib64/python2.6/site-packages/matplotlib/legend.pyc in _get_anchored_bbox(self, loc, bbox, parentbbox) 623 display coordinates. 624 """ --> 625 assert loc in range(1,11) # called only internally 626 627 BEST, UR, UL, LL, LR, R, CL, CR, LC, UC, C = range(11) AssertionError:
I presume my changes currently only allow loc as a location code. I didn't know that loc can be a tuple (axes coordinate I guess?). But it won't be hard to fix this. I'll work on it. -JJ On Tue, Dec 2, 2008 at 4:04 PM, Darren Dale <dsd...@gm...> wrote: > I think something broke with the recent changes to legend. For example, in > ipython -pylab: > > plot([1,2,3,4],label='test') > legend(loc=(.1, .5)) > > ERROR: An unexpected error occurred while tokenizing input > The following traceback may be corrupted or invalid > The error message is: ('EOF in multi-line statement', (185, 0)) > > ERROR: An unexpected error occurred while tokenizing input > The following traceback may be corrupted or invalid > The error message is: ('EOF in multi-line statement', (46, 0)) > > --------------------------------------------------------------------------- > AssertionError Traceback (most recent call last) > > /home/darren/<ipython console> in <module>() > > /usr/lib64/python2.6/site-packages/matplotlib/pyplot.pyc in legend(*args, > **kwargs) > > 2441 > 2442 ret = gca().legend(*args, > **kwargs) > -> 2443 > draw_if_interactive() > 2444 return > ret > 2445 if Axes.legend.__doc__ is not > None: > > /usr/lib64/python2.6/site-packages/matplotlib/backends/backend_qt4.pyc in > draw_if_interactive() > 38 figManager = > Gcf.get_active() > 39 if figManager != > None: > ---> 40 > figManager.canvas.draw() > > 41 > 42 def > _create_qApp(): > > /usr/lib64/python2.6/site-packages/matplotlib/backends/backend_qt4agg.pyc in > draw(self) > 131 if DEBUG: print "FigureCanvasQtAgg.draw", > self > 132 self.replot = > True > --> 133 > FigureCanvasAgg.draw(self) > 134 > self.update() > 135 # Added following line to improve realtime pan/zoom on > windows: > > > /usr/lib64/python2.6/site-packages/matplotlib/backends/backend_agg.pyc in > draw(self) > > 281 > 282 self.renderer = > self.get_renderer() > --> 283 > self.figure.draw(self.renderer) > > 284 > 285 def > get_renderer(self): > > /usr/lib64/python2.6/site-packages/matplotlib/figure.pyc in draw(self, > renderer) > > 770 > 771 # render the > axes > > --> 772 for a in self.axes: a.draw(renderer) > 773 > 774 # render the figure text > > > /usr/lib64/python2.6/site-packages/matplotlib/axes.pyc in draw(self, > renderer, > inframe) > > 1599 > 1600 for zorder, i, a in > dsu: > -> 1601 > a.draw(renderer) > > 1602 > 1603 > renderer.close_group('axes') > > /usr/lib64/python2.6/site-packages/matplotlib/legend.pyc in draw(self, > renderer) > 317 if > self._drawFrame: > 318 # update the location and size of the > legend > > --> 319 bbox = self._legend_box.get_window_extent(renderer) > 320 self.legendPatch.set_bounds(bbox.x0, bbox.y0, > 321 bbox.width, bbox.height) > > /usr/lib64/python2.6/site-packages/matplotlib/offsetbox.pyc in > get_window_extent(self, renderer) > 196 ''' > 197 w, h, xd, yd, offsets = self.get_extent_offsets(renderer) > --> 198 px, py = self.get_offset(w, h, xd, yd) > 199 return mtransforms.Bbox.from_bounds(px-xd, py-yd, w, h) > 200 > > /usr/lib64/python2.6/site-packages/matplotlib/offsetbox.pyc in > get_offset(self,width, height, xdescent, ydescent) > 155 """ > 156 if callable(self._offset): > --> 157 return self._offset(width, height, xdescent, ydescent) > 158 else: > 159 return self._offset > > /usr/lib64/python2.6/site-packages/matplotlib/legend.pyc in > _findoffset_loc(self, width, height, xdescent, ydescent) > 292 "Heper function to locate the legend" > 293 bbox = Bbox.from_bounds(0, 0, width, height) > --> 294 x, y = self._get_anchored_bbox(self._loc, bbox, > self.parent.bbox) > 295 return x+xdescent, y+ydescent > 296 > > /usr/lib64/python2.6/site-packages/matplotlib/legend.pyc in > _get_anchored_bbox(self, loc, bbox, parentbbox) > 623 display coordinates. > 624 """ > --> 625 assert loc in range(1,11) # called only internally > 626 > 627 BEST, UR, UL, LL, LR, R, CL, CR, LC, UC, C = range(11) > > AssertionError: > >
Darren, Can you test the attached patch and see if the legend is placed where you expected. Regards, -JJ
The patch is now applied to the SVN (r6479). -JJ On Tue, Dec 2, 2008 at 5:03 PM, Darren Dale <dsd...@gm...> wrote: > This looks right to me, thank you Jae-Joon. > > > On Tue, Dec 2, 2008 at 4:55 PM, Jae-Joon Lee <lee...@gm...> wrote: >> >> Darren, >> >> Can you test the attached patch and see if the legend is placed where >> you expected. >> Regards, >> >> -JJ > >
John, I just committed changes to SVN that reflect most of your comments. I didn't add the optional transformation support yet though. On Tue, Dec 2, 2008 at 7:45 AM, John Hunter <jd...@gm...> wrote: > On Mon, Dec 1, 2008 at 6:00 PM, Jae-Joon Lee <lee...@gm...> wrote: > >> As implementing an optional transformation should be trivial, I'll >> just go ahead an work on it. But it seems not clear to me how to >> approach this without any specific use case given. > > In another thread yesterday, someone was asking about a general way to > create a shadow effect. It occurred to me that we could probably use > the offsetbox for this. One could create an offset box for a given > artist that draws the artist offset from the original by 5 points or 5 > pixels, in a gray color that creates the appearance of a shadow. > Would this be a reasonable use case to test the offset transformation > in either points or pixels? > > JDH > I kind of prefer to use transforms.offset_copy() for simple offsets. But it may better to use some kind of container (e.g., offsetbox) if you have multiple artists. I'll think about it. Regards, -JJ
On Dec 4, 2008, at 7:21 PM, Jae-Joon Lee wrote: > John, > > I just committed changes to SVN that reflect most of your comments. > I didn't add the optional transformation support yet though. I'm getting a truncated line when calling: >>> plt.legend(numpoints=1) In the legend, I see a short dashed line followed by the marker. Before, the line went through the marker. I've attached a couple of images showing numpoints set to 1 and 2. I think this behavior was introduced with the improved legend class. Thanks, -Tony
Tony, I'll look at this problem. Anyhow, it seems to me that it happens because the handle length is too short. Can you try longer handle length and see what happens? The code for drawing handles are mostly identical to the previous one. Regards, -JJ On Fri, Dec 5, 2008 at 1:46 PM, Tony Yu <ts...@gm...> wrote: > > On Dec 4, 2008, at 7:21 PM, Jae-Joon Lee wrote: > >> John, >> >> I just committed changes to SVN that reflect most of your comments. >> I didn't add the optional transformation support yet though. > > > I'm getting a truncated line when calling: > >>>> plt.legend(numpoints=1) > > In the legend, I see a short dashed line followed by the marker. Before, the > line went through the marker. I've attached a couple of images showing > numpoints set to 1 and 2. > > I think this behavior was introduced with the improved legend class. > > Thanks, > -Tony > > > > > mpl svn 6497 >
Tony, Sorry. It turned to be a bug I introduced during the update. It should be fixed in r6499. Thanks, -JJ On Fri, Dec 5, 2008 at 3:47 PM, Jae-Joon Lee <lee...@gm...> wrote: > Tony, > > I'll look at this problem. > Anyhow, it seems to me that it happens because the handle length is too short. > Can you try longer handle length and see what happens? > The code for drawing handles are mostly identical to the previous one. > > Regards, > > -JJ > > > On Fri, Dec 5, 2008 at 1:46 PM, Tony Yu <ts...@gm...> wrote: >> >> On Dec 4, 2008, at 7:21 PM, Jae-Joon Lee wrote: >> >>> John, >>> >>> I just committed changes to SVN that reflect most of your comments. >>> I didn't add the optional transformation support yet though. >> >> >> I'm getting a truncated line when calling: >> >>>>> plt.legend(numpoints=1) >> >> In the legend, I see a short dashed line followed by the marker. Before, the >> line went through the marker. I've attached a couple of images showing >> numpoints set to 1 and 2. >> >> I think this behavior was introduced with the improved legend class. >> >> Thanks, >> -Tony >> >> >> >> >> mpl svn 6497 >> >