Opened 8 years ago

Closed 4 years ago

# Implement multi-function plotting options in plot()

Reported by: Owned by: alauve was minor sage-7.4 user interface plot, linestyle, color kcrisman, ppurka, jdemeyer, jason, saliola, jhonrubia6, paulmasson, novoselt Aaron Lauve Paul Masson N/A 4acb9d6 (Commits) 4acb9d65b9da7f60fb9e75c44344451e2b3f29ad

### Description (last modified by tscrim)

The code `plot([x,x^2],-.2,2)` is a nice shortcut for plotting both x and x^ 2 on the same graph. However, if you want to choose different colors or linestyles for each function, then it seems you have to execute, e.g.,

```p1=plot(x,-.2,1.2,color='blue',linestyle='dotted');
p2=plot(x^2,-.2,1.2,color='red',linestyle='dashed');
p1+p2
```

Here is how Maple and Mathematica handle this issue, respectively:

It seems that the keyword 'fill' allows for a dictionary input, so perhaps the relevant code there could serve as a template?

```[> plot([x, x^2], x= -.2..1.2, color=["blue", "red"],linestyle=[dot, longdash]);

In[1]:= Plot[{x, x^2}, {x, -.2, 1.2}, PlotStyle -> {{Blue, Dotted}, {Red, Dashed}}]
```

Remarks:

1. distinct choices for the curves should be made automatically for the user. Running, e.g.,
```In[1]:= Plot[{x, x^2}, {x, -.2, 1.2}]
```

in Mathematica produces two curves that are not both solid-blue.

1. the functionality for `fillcolor`, `color`, and `fill` should be the same insofar as possible.

### comment:1 Changed 7 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:2 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:3 Changed 6 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:4 Changed 6 years ago by vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:5 Changed 5 years ago by alauve

• Cc kcrisman ppurka added
• Description modified (diff)
• Milestone changed from sage-6.4 to sage-6.8

### comment:6 Changed 5 years ago by alauve

• Branch set to u/alauve/implement_multi_function_plotting_options_in_plot__

### comment:7 Changed 5 years ago by alauve

• Cc jdemeyer jason added
• Commit set to d169dc10c372f42287f4dddd3d75ceb5aceea4e3
• Status changed from new to needs_review

I've pushed a possible solution and set the ticket to 'needs_review'.

Comments?

New commits:

 ​d169dc1 `Started adding multiple directives for color and linestyle to _plot()`

### comment:8 Changed 5 years ago by git

• Commit changed from d169dc10c372f42287f4dddd3d75ceb5aceea4e3 to 2fb91728fd4c4b0f2f0cf4d2c1784d4c63a3b725

Branch pushed to git repo; I updated commit sha1. New commits:

 ​58f8323 `multi-function plotting now allows for and to be dictionaries/lists/tuples. Similar functionality added for and .` ​2fb9172 `deleted superfluous whitespace`

### comment:9 Changed 5 years ago by git

• Commit changed from 2fb91728fd4c4b0f2f0cf4d2c1784d4c63a3b725 to d02567ebfc875b4e6bf179c247429b31b1dbf6c8

Branch pushed to git repo; I updated commit sha1. New commits:

 ​d02567e `made plot-curve colors uniform, for ease of deciphering in lieu of a legend, when plot() is called with multiple functions.`

### comment:10 Changed 5 years ago by git

• Commit changed from d02567ebfc875b4e6bf179c247429b31b1dbf6c8 to ded10390c98b1f8cb29208debe5c4baa0319b392

Branch pushed to git repo; I updated commit sha1. New commits:

 ​c6ed638 `removed sphinx_plot calls, as I wasn't sure how to get them to work with doctests. (hope to put them back eventually.)` ​ded1039 `added some sphinx_plot calls`

### comment:11 Changed 5 years ago by git

• Commit changed from ded10390c98b1f8cb29208debe5c4baa0319b392 to f93aaf3543acd63a6783b5275cf6b5889c4d3080

Branch pushed to git repo; I updated commit sha1. New commits:

 ​f93aaf3 `Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__`

### comment:12 Changed 5 years ago by git

• Commit changed from f93aaf3543acd63a6783b5275cf6b5889c4d3080 to c9b1aa481c1a817899603255d05be7ad1aaadf36

Branch pushed to git repo; I updated commit sha1. New commits:

 ​c9b1aa4 `fixed a typo`

### comment:13 Changed 5 years ago by alauve

• Authors changed from Johan S. R. Nielsen to Aaron Lauve
• Cc saliola added
• Description modified (diff)

New commits:

 ​f93aaf3 `Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__` ​c9b1aa4 `fixed a typo`

### comment:14 Changed 5 years ago by kcrisman

Regarding the default coloring, please see also #8164.

### comment:15 Changed 5 years ago by git

• Commit changed from c9b1aa481c1a817899603255d05be7ad1aaadf36 to f7394a8a3d15dd4734a2bb13cdcae2b750641136

Branch pushed to git repo; I updated commit sha1. New commits:

 ​f7394a8 `Implemented new method for sampling colors from the spectrum for multi-function plotting.`

### comment:16 Changed 5 years ago by ppurka

I like what is done in this ticket. One of the first things I tried after seeing automatic colors and plots, is try to enable legend labels. I think it would be a good idea to also accept `legend_label` as a list and enable the legends in the same order as the multiple figures. Would it be too hard to do so?

### comment:17 Changed 5 years ago by alauve

Probably not. I'll look into it soon.

(What is beyond my powers is to do a better job of picking colors; see comments 15 and 16 at #8164.)

### comment:18 Changed 5 years ago by ppurka

Or, we can make a separate ticket for `legend_labels`. I am fine with this ticket as it is. Let me know which approach you prefer.

I have seen your comments in #8164. I agree that the colors could be chosen better, but that better be addressed in that ticket itself.

Otherwise there will be too much overlap between the tickets and it will take ages to merge anything. I have a feeling that getting consensus on the automatic color spectrum will take some time and discussion since different people tend to like different things. Some are worried about how the colors will look - how much they can be differentiated - when printed in grayscale, while some others are more concerned about how they will look on the screen.

### comment:19 Changed 5 years ago by kcrisman

Not to mention accessibility issues - Jeroen, if you see this, do you have any particular thoughts from your perspective?

### comment:20 Changed 4 years ago by tscrim

• Branch changed from u/alauve/implement_multi_function_plotting_options_in_plot__ to public/graphs/multi_function_plot_options-12962
• Cc jhonrubia6 paulmasson added
• Commit changed from f7394a8a3d15dd4734a2bb13cdcae2b750641136 to eb224ab0fa489d2444d179a4d78574559b52ec97
• Description modified (diff)
• Milestone changed from sage-6.8 to sage-7.3

Just rebased the current branch, but it would be good to have this functionality.

New commits:

 ​eb224ab `Merge branch 'u/alauve/implement_multi_function_plotting_options_in_plot__' of trac.sagemath.org:sage into public/graphs/multi_function_plot_options-12962`

### comment:21 Changed 4 years ago by tscrim

• Branch changed from public/graphs/multi_function_plot_options-12962 to public/graphics/multi_function_plot_options-12962
• Commit eb224ab0fa489d2444d179a4d78574559b52ec97 deleted

### comment:22 Changed 4 years ago by git

• Commit set to eb224ab0fa489d2444d179a4d78574559b52ec97

Branch pushed to git repo; I updated commit sha1. New commits:

 ​d169dc1 `Started adding multiple directives for color and linestyle to _plot()` ​58f8323 `multi-function plotting now allows for and to be dictionaries/lists/tuples. Similar functionality added for and .` ​2fb9172 `deleted superfluous whitespace` ​d02567e `made plot-curve colors uniform, for ease of deciphering in lieu of a legend, when plot() is called with multiple functions.` ​c6ed638 `removed sphinx_plot calls, as I wasn't sure how to get them to work with doctests. (hope to put them back eventually.)` ​ded1039 `added some sphinx_plot calls` ​f93aaf3 `Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__` ​c9b1aa4 `fixed a typo` ​f7394a8 `Implemented new method for sampling colors from the spectrum for multi-function plotting.` ​eb224ab `Merge branch 'u/alauve/implement_multi_function_plotting_options_in_plot__' of trac.sagemath.org:sage into public/graphs/multi_function_plot_options-12962`

### comment:23 Changed 4 years ago by paulmasson

• Reviewers set to Paul Masson
• Status changed from needs_review to needs_work

Doctests failing with message

```Error: TAB character found at lines 1089,1092,1099
[383 tests, 47.56 s]
```

Building documentation produces too many errors to copy here.

### comment:24 Changed 4 years ago by git

• Commit changed from eb224ab0fa489d2444d179a4d78574559b52ec97 to fe05d0ab2a577acc0f2b93cb5d7e92989040cc81

Branch pushed to git repo; I updated commit sha1. New commits:

 ​fe05d0a `zapped the TAB characters`

### comment:25 Changed 4 years ago by kcrisman

Travis and Paul,

Just be sure to check out some of the discussion here and #8164 regarding the auto-coloring options. I'm still somewhat reluctant to change the default due to accessibility and 'backwards compatibility' (and that the yellow and a couple others are harder to see), since it's not like we are doing proper release notes and this may surprise some ... (There was also discussion of whether `rainbow()` is the best choice for multicolor for other reasons but I'll stay out of that.) Even a scale of different blues might be better than that.

### comment:26 follow-up: ↓ 30 Changed 4 years ago by paulmasson

Karl-Dieter, since you appear to be the only developer disagreeing about having automatic colors, is there some way to solicit input on this question from users? Does Sage have some sort of user mailing list for sending out questions or making announcements of this sort? What about an open question on ask.sagemath.org?

I personally like the rainbow colors, and this version is actually better than Mathematica's.

### comment:27 follow-up: ↓ 37 Changed 4 years ago by paulmasson

On matters of code, the doctests now pass but documentation building is still a problem, with at least two plots failing. Aaron, please try building the documentation to see where the errors are.

One feature that I do not like in this branch is that `linestyle` defaults to automatic. I find solid lines much easier to read and would prefer that `linestyle` default to solid. That is in line with Mathematica behavior.

Also, if both `rgbcolor` and `color` are specified, this branch silently overrides the former with the latter. In the current version of `plot.py`, specifying both of these options leads to a `RuntimeError` coming from the current line 780:

```@rename_keyword(color='rgbcolor')
```

I was in the future planning to remove that line and add code to raise a `ValueError` as Travis had me do in #9654. I think this branch should raise the `ValueError` rather than silently override an option, and then this decorator can be removed.

### comment:28 Changed 4 years ago by paulmasson

• Cc novoselt added

### comment:29 Changed 4 years ago by novoselt

After reading all comments but not studying the code:

• I most definitely agree that `linestyle` should be a solid line for all unless changed by a user;
• I don't think we should be too concerned with making "proper choice" for default different colors: the current situation is that they all are the same and users who care about multiple functions probably have multiple plot commands with custom colors. Hence we will not affect/break old code as long as the default for a single plot is the same/similar.

### comment:30 in reply to: ↑ 26 Changed 4 years ago by kcrisman

• Commit fe05d0ab2a577acc0f2b93cb5d7e92989040cc81 deleted

Karl-Dieter, since you appear to be the only developer disagreeing about having automatic colors, is there some way to solicit input on this question from users? Does Sage have some sort of user mailing list for sending out questions or making announcements of this sort? What about an open question on ask.sagemath.org?

Usually in this case one of two things is done (I don't think this is an "official" policy, though):

• Ask on sage-devel if anyone cares
• Majority rule on the ticket

Sometimes it does matter enough, but even then usually on sage-devel only a few people will pipe up unless it's something that affects a fairly broad range of people.

In this case, I don't care strongly enough to block this now that several people have 'officially' weighed in (including on #8164). You may still want to look there regarding different options for auto-coloring - matplotlib had a huge and long debate over their new default colormap for best visualization of data. But in any case once it's decided here, you should close #8164 as a dup.

### comment:31 Changed 4 years ago by chapoton

• Branch public/graphics/multi_function_plot_options-12962 deleted

public/graphics/multi_function_plot_options-12962

was not a correct git branch name, so I removed it

### comment:32 Changed 4 years ago by tscrim

• Branch set to public/graphics/multi_function_plot_options-12962
• Commit set to 276f8eb2f08a898e6be8b20c09dd9c5b4f49a5b0
• Status changed from needs_work to needs_review

I have no idea about what happened to the branch...strange...

My 2 cents: I think having the default linestyle being a solid color is what we should do. I haven't really looked into the code, but just having the option of providing multiple colors is a huge +1 for including this (and is a natural thing for people to try when doing multiple plots).

New commits:

 ​d169dc1 `Started adding multiple directives for color and linestyle to _plot()` ​58f8323 `multi-function plotting now allows for and to be dictionaries/lists/tuples. Similar functionality added for and .` ​2fb9172 `deleted superfluous whitespace` ​d02567e `made plot-curve colors uniform, for ease of deciphering in lieu of a legend, when plot() is called with multiple functions.` ​c6ed638 `removed sphinx_plot calls, as I wasn't sure how to get them to work with doctests. (hope to put them back eventually.)` ​ded1039 `added some sphinx_plot calls` ​f93aaf3 `Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__` ​c9b1aa4 `fixed a typo` ​f7394a8 `Implemented new method for sampling colors from the spectrum for multi-function plotting.` ​276f8eb `Merge branch 'u/alauve/implement_multi_function_plotting_options_in_plot__' of trac.sagemath.org:sage into public/graphics/multi_function_plot_options-12962`

### comment:33 Changed 4 years ago by alauve

• Status changed from needs_review to needs_work

Sorry, Travis, Not quite ready for review... I think there are one or two points to address in the preceding comments. Expect to see me revert it to 'needs review' later in the week.

• accept `legend_label` as a list
• restore `linestyle` default back to 'solid'
• implement `ValueError` when using two color models
• build documentation to find failing (sphinx?) plots

Stay tuned

### comment:34 Changed 4 years ago by alauve

Karl-Dieter, I found a nice website that will take a url and pass its content through assorted "color blind" filters. They do b&w too. Follow the instructions here: color test

It seems that advancing colors on the hue scale by 1/(golden ratio) works well in all cases, so I'm going with this for now. One can always start a new ticket to implement more advanced color theory methods. (In particular, I have no idea how to bring the matplotlib 2.0 colormaps into the picture.)

### comment:35 Changed 4 years ago by git

• Commit changed from 276f8eb2f08a898e6be8b20c09dd9c5b4f49a5b0 to 02f19a24c59dd2a95801282725909f63924c3340

Branch pushed to git repo; I updated commit sha1. New commits:

 ​02f19a2 `Made default color choices when plot() is passed a list of functions.`

### comment:36 Changed 4 years ago by paulmasson

One doctest failing with error message:

```File "src/sage/plot/plot.py", line 1967, in sage.plot.plot.?
Failed example:
len((q1).matplotlib().axes[0].legend().texts) # used to raise AttributeError
Expected:
1
Got:
2
```

### comment:37 in reply to: ↑ 27 Changed 4 years ago by alauve

Hmm..

Okay, I confirm the `RuntimeError` when both `color` and `rgbcolor` are specified. But I'm not sure I understand what you are proposing.

• Do you want a `ValueError` if the keyword `color` is used at all? (or if you prefer, `rgbcolor` is used?)
• Or do you want a `ValueError` if both are specified? I don't know why anybody would do this, but I also don't have an objection to adding such a check.

Is there any reason to have both keywords floating around? Perhaps the simplest solution is for me to replace all instances of `rgbcolor` with `color` within `plot.py`? (When I first read this code, I assumed the `@rename_keyword` call was a generous short-cut provided to the end-user by the code-writer. I certainly don't want to write `rgbcolor=` if I don't have to.)

Replying to paulmasson:

On matters of code, the doctests now pass but documentation building is still a problem, with at least two plots failing. Aaron, please try building the documentation to see where the errors are.

One feature that I do not like in this branch is that `linestyle` defaults to automatic. I find solid lines much easier to read and would prefer that `linestyle` default to solid. That is in line with Mathematica behavior.

Also, if both `rgbcolor` and `color` are specified, this branch silently overrides the former with the latter. In the current version of `plot.py`, specifying both of these options leads to a `RuntimeError` coming from the current line 780:

```@rename_keyword(color='rgbcolor')
```

I was in the future planning to remove that line and add code to raise a `ValueError` as Travis had me do in #9654. I think this branch should raise the `ValueError` rather than silently override an option, and then this decorator can be removed.

### comment:38 Changed 4 years ago by paulmasson

The `@rename_keyword` decorator in this file first appeared in #4201. On the same ticket this decorator and `@options` were added to `misc/decorators.py`. The description for the latter is about the convenience of the end user, so I think we can assume the same for the former.

The functions in a plot get passed to `line.py` as subplots, and the `_allowed_options` for lines include `rgbcolor` but not `color`. We can't just rename keywords here that are expected to have a certain name elsewhere, unless you want to track down every plot file that will get output from `plot.py` and change all of them.

What Travis had me do in #9654 was raise the `ValueError` only if both keywords were used at the same time. This lets the end user input either keyword as desired but avoids conflicting information that might confuse.

On #9654 color values were passed to another keyword, so I did the check to make sure only one was present and set the new keyword value. For this ticket `rgbcolor` is expected in other places, so after checking that only one is present you could use presumably use the function `rename_keyword(color='rgbcolor')` when `color` is present to copy over values, but then always use the keyword `rgbcolor` when passing values elsewhere.

This reply is bit long-winded, but I'd like to have a record of some of the details of how plotting works in Sage. Let me also know if I've not yet been clear enough.

### comment:39 Changed 4 years ago by git

• Commit changed from 02f19a24c59dd2a95801282725909f63924c3340 to 590325e590189723fb38174a159afddc223e37fc

Branch pushed to git repo; I updated commit sha1. New commits:

 ​590325e `doctests pass; ValueError check works if color and rgbcolor are both passed; errors remain in building documentation`

### comment:40 follow-up: ↓ 42 Changed 4 years ago by paulmasson

Aaron, a couple minor things:

1) When the plot/subplot is created in what is currently L1994, please use the keyword `rgbcolor` rather than `color` for consistency with other plot files.

2) Most files I've seen in Sage don't have spaces around the equals sign when assigning keyword arguments. Please remove the extra white space for consistency with other files.

Thanks!

### comment:41 Changed 4 years ago by git

• Commit changed from 590325e590189723fb38174a159afddc223e37fc to c836728856ed1c419f7bd53fa28e2ac57a941713

Branch pushed to git repo; I updated commit sha1. New commits:

 ​7d593cd `removed whitespace around keyword calls` ​c836728 `working toward request to use rgbcolor instead of color. Simply making this change in L2139 breaks the plot()`

### comment:42 in reply to: ↑ 40 ; follow-up: ↓ 50 Changed 4 years ago by alauve

Replying to paulmasson:

Aaron, a couple minor things:

1) When the plot/subplot is created in what is currently L1994, please use the keyword `rgbcolor` rather than `color` for consistency with other plot files.

ran into a problem naively implementing this. Simply replacing `color` with `rgbcolor` on L2139 (in current ticket) breaks Sage's plot() function. I'm not sure I can track down the problem, but I'll give it a try tomorrow.

2) Most files I've seen in Sage don't have spaces around the equals sign when assigning keyword arguments. Please remove the extra white space for consistency with other files.

done.

### comment:43 Changed 4 years ago by git

• Commit changed from c836728856ed1c419f7bd53fa28e2ac57a941713 to dc1561813b407f70c433eeb0bacd6f7d6e5721ac

Branch pushed to git repo; I updated commit sha1. New commits:

 ​dc15618 `documentation now builds. problem was attempting to set frame and axes options in graphics_array with sphinx_plot.`

### comment:44 Changed 4 years ago by alauve

A remark on the documentation fix mentioned above. This is what I want to mimic with sphinx_plot:

```        sage: p1 = plot([sin(x), cos(2*x)*sin(x)], -pi, pi, fill={1: [0]}, fillcolor='blue', fillalpha=.25, color='blue')
sage: p2 = plot([sin(x), cos(2*x)*sin(x)], -pi, pi, fill={0: 1, 1:[0]}, color=['blue'])
sage: p3 = plot([sin(x), cos(2*x)*sin(x)], -pi, pi, fill=[0, [0]], fillcolor=['#f60'], fillalpha=1, color={1: 'blue'})
sage: p4 = plot([sin(x), cos(2*x)*sin(x)], (x,-pi, pi), fill=[0, x/pi], fillcolor='grey', color=['red', 'blue'])
sage: graphics_array([[p1, p2], [p3, p4]]).show(frame=True, axes=False) # long time
```

I'm not quite sure how to get frame and axes options passed to sphinx_plot:

Any ideas?

### comment:45 Changed 4 years ago by git

• Commit changed from dc1561813b407f70c433eeb0bacd6f7d6e5721ac to 165dda5ed454d0f684a937eb997c08e82d47d6ab

Branch pushed to git repo; I updated commit sha1. New commits:

 ​165dda5 `cleaned up a few comments; deleted others no longer needed`

### comment:46 Changed 4 years ago by alauve

• Status changed from needs_work to needs_review

### comment:47 Changed 4 years ago by tscrim

Unfortunately, I don't think `GraphicsArray` currently has the capabilities to set options like `axes` and `frame`.

### comment:48 Changed 4 years ago by kcrisman

See e.g. #10657 and #11160.

### comment:49 Changed 4 years ago by jhonrubia6

You can hack the axes part thanks to the `axes` method in `Graphics`, I've used this trick in other tickets

```p1 = plot([sin(x), cos(2*x)*sin(x)], -pi, pi, fill={1: [0]}, fillcolor='blue', fillalpha=.25, color='blue')
p2 = plot([sin(x), cos(2*x)*sin(x)], -pi, pi, fill={0: 1, 1:[0]}, color=['blue'])
p3 = plot([sin(x), cos(2*x)*sin(x)], -pi, pi, fill=[0, [0]], fillcolor=['#f60'], fillalpha=1, color={1: 'blue'})
p4 = plot([sin(x), cos(2*x)*sin(x)], (x,-pi, pi), fill=[0, x/pi], fillcolor='grey', color=['red', 'blue'])
p1.axes(show=False);p2.axes(show=False);p3.axes(show=False);p4.axes(show=False)
sphinx_plot(graphics_array([[p1, p2], [p3, p4]]))
```

Unfortunately we don't have a `frame` method neither in `Graphics` nor in `GraphicsArray`

### comment:50 in reply to: ↑ 42 ; follow-up: ↓ 51 Changed 4 years ago by paulmasson

Replying to alauve:

Replying to paulmasson:

Aaron, a couple minor things:

1) When the plot/subplot is created in what is currently L1994, please use the keyword `rgbcolor` rather than `color` for consistency with other plot files.

ran into a problem naively implementing this. Simply replacing `color` with `rgbcolor` on L2139 (in current ticket) breaks Sage's plot() function. I'm not sure I can track down the problem, but I'll give it a try tomorrow.

`plot()` works for me with this change. Do you still consider this a problem?

2) Most files I've seen in Sage don't have spaces around the equals sign when assigning keyword arguments. Please remove the extra white space for consistency with other files.

done.

Thanks.

### comment:51 in reply to: ↑ 50 Changed 4 years ago by alauve

Replying to paulmasson:

Replying to alauve:

Replying to paulmasson:

Aaron, a couple minor things:

1) When the plot/subplot is created in what is currently L1994, please use the keyword `rgbcolor` rather than `color` for consistency with other plot files.

ran into a problem naively implementing this. Simply replacing `color` with `rgbcolor` on L2139 (in current ticket) breaks Sage's plot() function. I'm not sure I can track down the problem, but I'll give it a try tomorrow.

`plot()` works for me with this change. Do you still consider this a problem?

Nope. All is well. I must have made a complementary change since that time.

2) Most files I've seen in Sage don't have spaces around the equals sign when assigning keyword arguments. Please remove the extra white space for consistency with other files.

done.

Thanks.

### comment:52 follow-up: ↓ 53 Changed 4 years ago by paulmasson

Doctests pass. Documentation builds and looks good as is.

Aaron, are you still planning to work on plots on this ticket or can I set positive review?

### comment:53 in reply to: ↑ 52 Changed 4 years ago by alauve

Hmm, Looking over the documentation, I see a few minor changes that I should make. I'll toggle back to needs_work. When you see it return to needs_review, I'm done.

Thx.

Replying to paulmasson:

Doctests pass. Documentation builds and looks good as is.

Aaron, are you still planning to work on plots on this ticket or can I set positive review?

### comment:54 Changed 4 years ago by alauve

• Status changed from needs_review to needs_work

### comment:55 Changed 4 years ago by git

• Commit changed from 165dda5ed454d0f684a937eb997c08e82d47d6ab to 26ac1d3ef790319e5cedce5a3cd6ddb1242e943b

Branch pushed to git repo; I updated commit sha1. New commits:

 ​26ac1d3 `added a few more sphinx_plots; better described how default colors are chosen`

### comment:56 Changed 4 years ago by alauve

• Status changed from needs_work to needs_review

### comment:57 follow-up: ↓ 59 Changed 4 years ago by paulmasson

Aaron, you added a bit more that what's in your description: the options for `legend_color` are expanded which is good and functions as expected.

Regarding legends, for the case of multiple graph colors with a single `legend_label`, I find the empty spaces in the legend unexpected. Wouldn't it make more sense to repeat the single label for all lines? If I'm graphing multiple functions and deliberately give one label, I'd expect it to appear for all of them.

### comment:58 Changed 4 years ago by paulmasson

Also, the documentation for `legend_color` and `fillcolor` says that they follow the same conventions as `color`. That's not completely true, since the default for `legend_color` is black and the default for `fillcolor` is gray. Please add those defaults for clarity.

Last edited 4 years ago by paulmasson (previous) (diff)

### comment:59 in reply to: ↑ 57 Changed 4 years ago by alauve

Replying to paulmasson:

Aaron, you added a bit more that what's in your description: the options for `legend_color` are expanded which is good and functions as expected.

ah yes... it occurred to me that this was a natural extension of what was asked for in 16. My apologies for not mentioning it.

Regarding legends, for the case of multiple graph colors with a single `legend_label`, I find the empty spaces in the legend unexpected. Wouldn't it make more sense to repeat the single label for all lines? If I'm graphing multiple functions and deliberately give one label, I'd expect it to appear for all of them.

I don't know that I agree. I was trying to think of all the reasons I'd just include one label... "trig functions" or "cos(nx)" or some such. I don't think I'd want to see that written multiple times. Certainly, seeing "f" or "x2" written multiple times would be silly. But, it is nearly as easy to write ["trig functions", "", ""] as it is to write "trig functions", so I'm happy to modify the code accordingly.

### comment:60 Changed 4 years ago by git

• Commit changed from 26ac1d3ef790319e5cedce5a3cd6ddb1242e943b to 742422e65b2bc8321bb784bd5229ce12c08873dd

Branch pushed to git repo; I updated commit sha1. New commits:

 ​742422e `Modified behavior of fillcolor and legend_label, and reworked some of the sphinx_plot examples as well.`

### comment:61 Changed 4 years ago by paulmasson

• Milestone changed from sage-7.3 to sage-7.4
• Status changed from needs_review to positive_review

Good to go.

### comment:62 Changed 4 years ago by vbraun

• Commit 742422e65b2bc8321bb784bd5229ce12c08873dd deleted
• Status changed from positive_review to needs_work

Branch seems to be missing?

### comment:63 Changed 4 years ago by git

• Commit set to 4acb9d65b9da7f60fb9e75c44344451e2b3f29ad

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

 ​276f8eb `Merge branch 'u/alauve/implement_multi_function_plotting_options_in_plot__' of trac.sagemath.org:sage into public/graphics/multi_function_plot_options-12962` ​02f19a2 `Made default color choices when plot() is passed a list of functions.` ​590325e `doctests pass; ValueError check works if color and rgbcolor are both passed; errors remain in building documentation` ​7d593cd `removed whitespace around keyword calls` ​c836728 `working toward request to use rgbcolor instead of color. Simply making this change in L2139 breaks the plot()` ​dc15618 `documentation now builds. problem was attempting to set frame and axes options in graphics_array with sphinx_plot.` ​165dda5 `cleaned up a few comments; deleted others no longer needed` ​26ac1d3 `added a few more sphinx_plots; better described how default colors are chosen` ​742422e `Modified behavior of fillcolor and legend_label, and reworked some of the sphinx_plot examples as well.` ​4acb9d6 `Merge branch 'public/graphics/multi_function_plot_options-12962' of trac.sagemath.org:sage into public/graphics/multi_function_plot_options-12962`

### comment:64 Changed 4 years ago by tscrim

• Status changed from needs_work to positive_review

Hmm...strange. I have the most recent copy, so I've pushed it back.

### comment:65 Changed 4 years ago by vbraun

• Branch changed from public/graphics/multi_function_plot_options-12962 to 4acb9d65b9da7f60fb9e75c44344451e2b3f29ad
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.