[with patch; positive review] Add legends to plot.py

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.

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.

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

1.

-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

2.

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

3.

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

4.

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

mvngu's doc fixes

Replying to mvngu:

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

1. {{{ -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 }}} 2. {{{ -If called with no input return the current legend setting. +If called with no input, return the current legend setting. }}} 3. {{{ -Sets the legend label font +Sets the legend label font. }}} 4. {{{ -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 :)

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.

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

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).

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

Cheers,

Michael

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.

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.

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

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!!