Add legends to plot.py

[sagetrac-anakha]

Add support for placing legends on plots using the matplotlib facility for doing so.

CC: @kcrisman @sagetrac-slosoi @novoselt @sagetrac-abergeron @jhpalmieri @sagetrac-mvngu

Component: graphics

Author: Arnaud Bergeron, Karl-Dieter Crisman

Reviewer: Jason Grout, Mike Hansen, Karl-Dieter Crisman

Merged: sage-4.6.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/4342

[sagetrac-anakha]

Attachment: trac_4342.patch.gz

[sagetrac-anakha]

comment:1

Add the option to put legends on plots.

Also fixes three long test markers that where reading #long rather than #long time and two or three minor documentation mistakes elsewhere in the code. These where bundled because they don't warrant a separate ticket for me but are still nice to fix.

[sagetrac-anakha]

comment:2

New patch to pass options around in a better way. Also now use @suboptions marker from #4203 so you need to apply that patch first.

[sagetrac-anakha]

Attachment: trac_4342-2.patch.gz

[sagetrac-mvngu]

comment:3

For the patch trac_4342.patch, here are possible fixes to your documentation:

-denoting a color or an rgb tuple. The string can be a color name
+denoting a color or an RGB tuple. The string can be a color name

-If called with no input return the current legend setting.
+If called with no input, return the current legend setting.

-Sets the legend label font
+Sets the legend label font.

-form is just a floating point rgb tuple with all values ranging
+form is just a floating point RGB tuple with all values ranging

[TimothyClemans]

Attachment: trac_4342-3.patch.gz

mvngu's doc fixes

[TimothyClemans]

comment:4

Replying to @sagetrac-mvngu:

For the patch trac_4342.patch, here are possible fixes to your documentation:

-denoting a color or an rgb tuple. The string can be a color name
+denoting a color or an RGB tuple. The string can be a color name

-If called with no input return the current legend setting.
+If called with no input, return the current legend setting.

-Sets the legend label font
+Sets the legend label font.

-form is just a floating point rgb tuple with all values ranging
+form is just a floating point RGB tuple with all values ranging

I uploaded a patch with these fixes.

Mvngu, in the future please upload patches with your doc fixes. Thanks :)

[mwhansen]

comment:5

Attachment: trac_4342.2.patch.gz

I've folded all the patches together and rebased them against plot.py refactoring. These are in trac_4342.2.patch .

I'll try to give this a proper review sometime this weekend.

[jasongrout]

comment:6

what's the status on this? Mike, did you have time to look at it?

[wdjoyner]

comment:7

The good news is that Mike Hansen's patch does apply cleanly to 3.2.2.

The bad news is that this patch breaks some basic functionality:

sage: p = plot(tan(x), x, -1/2, 1/2)
sage: show(p)

raises a TypeError exception.

That is serious but more serious (to me - maybe this exception is
an easy-to-fix bug) is the design of this legend functionality.

It seems to me (and people who know about the Graphics class can hopefully
correct me if I am wrong) the following is both possible and a fairly
natural way of writing a legend as another graphics object:

1. given 2d plot objects P, Q (ie, instances of a Graphics class),
   one could wrote a function which returns
   (a) the functions being plotted in P,Q (resp),
   (b) the line style used in P,Q (resp).
   From this data, one can write a function taking P,Q (and a cnter point)
   as arguments (more generally any list of such plot objects) and returning
   a legend as an analog of a text plot. This can be placed anywhere
   by choosing the center point appropriately.

I'm happy to change my opinion (since IMHO this added functionality would be great),
especially if someone with more knowledge of the plotting can comment positively
on this. I really greatly appreciate the hard programming work that went into this.
Maybe I'm being naive but the design to me seems a bit awkward, and in any case
it breaks show (which is bad, hence making it a "show stopper", if you excuse
the horribly bad pun).

[sagetrac-mabshoff]

comment:8

There is no such thing as "negative review", but there is "needs work".

Cheers,

Michael

[sagetrac-abergeron]

Attachment: trac_4342_v3.patch.gz

[sagetrac-abergeron]

comment:9

Attachment: trac_4342_v3.2.patch.gz

The last patch (trac_4342_v3.2.patch) fixes the TypeError problem and add tests and documentation where appropriate (can't believe I missed these).

As far as I know, there is no way to get the function that's being plotted from the GraphicsPrimitve object (except maybe for some specialized primitives like Circle) because all you have is a matrix of sample points. So I don't think it is a good way.

About the center point, it is already possible to specify a tuple of floats between 0 and 1 that gives the relative coordinates of the legend box.

And last, about the design, I kind of copied it from matplotlib where each object is labeled beforehand. I could always provide an option to specify legend labels in show() and save() but due to the design of Graphics I think this would be shaky at best because the user has now reliable way of knowing the content of a Graphics object.

Anyway if some clever way of overcoming the obstacle(s) above is found then something like what you want could be implemented in a later patch.

[wdjoyner]

comment:10

Can you please explain exactly which patches to apply and in which order from a new clone of Sage? I can't get them to apply cleanly.

[sagetrac-abergeron]

comment:11

You only need trac_4342_v3.2.patch. Ignore all others.

[wdjoyner]

comment:12

Again, I am having trouble running tests, so I can't say if they all pass. The examples seem fine (if somewhat too few). Still, which this very clever bit of programmming, this block of commands

sage: P1 = plot(exp(x), 0, 2, legend_label='$y=e^x$')
sage: P2 = plot(sin(x), 0, 2, rgbcolor=(1,0,0),legend_label='$y=\sin(x)$')
sage: show(P1+P2)

produces a beautiful plot of two curves, one red and the other blue, and the legend labels are collected into a distinctive light grey box with the correct labels.

Wonderful!!

[sagetrac-mhampton]

comment:36

This applies cleanly against sage-4.6.alpha0, and most things seem to work well.

I'm not sure I understand Jason's point about the suboptions and contour_plot. It does seem like that while contour_plot accepts the legend_label argument, it doesn't do anything. I'm not sure that's a deal-breaker though, maybe we can just add a note in the contour_plot docstring about that given contour_plot's nice sidebar functionality.

At least on my system (mac 10.5, firefox), font_style='italic' doesn't do anything. Other font options seem to work though.

Since the default behavior is to have this turned off, I am hoping this can get a positive review pretty soon. It may not be perfect but it seems good enough for incremental improvement. I will not give it a positive review myself yet since I don't understand all the issues.

[jasongrout]

comment:37

I browsed through the patch, and things look okay. I'm downloading sage 4.6alpha so I can try it out.

[jasongrout]

comment:38

I rebased the patch for 4.6.alpha1 (depends on #9740 and #9746). I also fixed a few small doc formatting issues.

I'm going to doctest it all now.

[jasongrout]

comment:39

When I build the docs sage -docbuild reference html, I get

/Users/grout/sage/devel/sage/doc/en/reference/sage/plot/plot.rst:1070: (ERROR/3) Unknown target name: "legend".

I can't find where this error is.

[kcrisman]

comment:40

I can't give this a positive review since I helped write the last iteration.

I am not sure what's going on with the docbuild either. reference/plotting.rst is where things come from, but it refers of course to all the usual files.

[jasongrout]

Reviewer: Jason Grout

[kcrisman]

Author: Arnaud Bergeron, Karl-Dieter Crisman

[kcrisman]

comment:42

I think that username anakha is really abergeron in a previous username incarnation. Here is one example.

[kcrisman]

Changed reviewer from Jason Grout to Jason Grout, Mike Hansen, Karl-Dieter Crisman

[jasongrout]

comment:43

Okay, once we figure out where that problem with plotting.rst is, positive review.

ptestlong passes with the following tickets applied in order: #9221 (and new spkg), #9740, #9746, #4342.

[jasongrout]

comment:44

CCing two sphinx experts. When we apply trac-4342-rebase-4.6.alpha1.patch (only), we get the following error when building docs:

/Users/grout/sage/devel/sage/doc/en/reference/sage/plot/plot.rst:1070: (ERROR/3) Unknown target name: "legend".

We don't know where it is coming from. Do you guys have any idea?

Please note that this ticket depends on #9740, then #9746.

[jhpalmieri]

comment:45

In the line

        - ``legend_*`` - all the options valid for :meth:`set_legend_options` prefixed with 'legend_'

you need to change 'legend_' to ``legend_``. Sphinx (maybe ReST?) gives special meaning to strings ending in underscores.

[jasongrout]

comment:46

ah, those are citation references, aren't they? Thanks!

[jasongrout]

comment:47

John's fix is in the last patch.

Release manager: please merge trac-4342-rebase-4.6.alpha1.patch, then 4342-fix-docs.patch.

Positive review. Good job, everyone! Sorry this took so long to review!

[jasongrout]

Attachment: trac-4342-rebase-4.6.alpha1.patch.gz

apply only this patch; rebased to 4.6.alpha1

[jasongrout]

Attachment: 4342-fix-docs.patch.gz

apply on top of previous patches

[jasongrout]

comment:48

(I rebased to account for some changes on #9740)

[kcrisman]

comment:49

Replying to @jasongrout:

(I rebased to account for some changes on #9740)

Correct - see the ticket order a few comments ago. Applies fine to the latest changes at #9740.

[qed777]

comment:50

Do #9740, #9746 and #4342 depend on #9221?

[jasongrout]

comment:51

Replying to @qed777:

Do #9740, #9746 and #4342 depend on #9221?

I don't think so. I originally tested them all without #9221.

[qed777]

Merged: sage-4.6.alpha3