Opened 5 years ago

Closed 14 months ago

#12962 closed enhancement (fixed)

Implement multi-function plotting options in plot()

Reported by: alauve Owned by: was
Priority: minor Milestone: sage-7.4
Component: user interface Keywords: plot, linestyle, color
Cc: kcrisman, ppurka, jdemeyer, jason, saliola, jhonrubia6, paulmasson, novoselt Merged in:
Authors: Aaron Lauve Reviewers: Paul Masson
Report Upstream: N/A Work issues:
Branch: 4acb9d6 (Commits) Commit: 4acb9d65b9da7f60fb9e75c44344451e2b3f29ad
Dependencies: Stopgaps:

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.

Change History (65)

comment:1 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:2 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:3 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:4 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 2 years ago by alauve

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

comment:6 Changed 2 years ago by alauve

  • Branch set to u/alauve/implement_multi_function_plotting_options_in_plot__

comment:7 Changed 2 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:

d169dc1Started adding multiple directives for color and linestyle to _plot()

comment:8 Changed 2 years ago by git

  • Commit changed from d169dc10c372f42287f4dddd3d75ceb5aceea4e3 to 2fb91728fd4c4b0f2f0cf4d2c1784d4c63a3b725

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

58f8323multi-function plotting now allows for and to be dictionaries/lists/tuples. Similar functionality added for and .
2fb9172deleted superfluous whitespace

comment:9 Changed 2 years ago by git

  • Commit changed from 2fb91728fd4c4b0f2f0cf4d2c1784d4c63a3b725 to d02567ebfc875b4e6bf179c247429b31b1dbf6c8

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

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

comment:10 Changed 2 years ago by git

  • Commit changed from d02567ebfc875b4e6bf179c247429b31b1dbf6c8 to ded10390c98b1f8cb29208debe5c4baa0319b392

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

c6ed638removed sphinx_plot calls, as I wasn't sure how to get them to work with doctests. (hope to put them back eventually.)
ded1039added some sphinx_plot calls

comment:11 Changed 2 years ago by git

  • Commit changed from ded10390c98b1f8cb29208debe5c4baa0319b392 to f93aaf3543acd63a6783b5275cf6b5889c4d3080

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

f93aaf3Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__

comment:12 Changed 2 years ago by git

  • Commit changed from f93aaf3543acd63a6783b5275cf6b5889c4d3080 to c9b1aa481c1a817899603255d05be7ad1aaadf36

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

c9b1aa4fixed a typo

comment:13 Changed 2 years ago by alauve

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

New commits:

f93aaf3Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__
c9b1aa4fixed a typo

comment:14 Changed 2 years ago by kcrisman

Regarding the default coloring, please see also #8164.

comment:15 Changed 2 years ago by git

  • Commit changed from c9b1aa481c1a817899603255d05be7ad1aaadf36 to f7394a8a3d15dd4734a2bb13cdcae2b750641136

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

f7394a8Implemented new method for sampling colors from the spectrum for multi-function plotting.

comment:16 Changed 2 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 2 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 2 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 2 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 15 months 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:

eb224abMerge 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 15 months 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 15 months ago by git

  • Commit set to eb224ab0fa489d2444d179a4d78574559b52ec97

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

d169dc1Started adding multiple directives for color and linestyle to _plot()
58f8323multi-function plotting now allows for and to be dictionaries/lists/tuples. Similar functionality added for and .
2fb9172deleted superfluous whitespace
d02567emade plot-curve colors uniform, for ease of deciphering in lieu of a legend, when plot() is called with multiple functions.
c6ed638removed sphinx_plot calls, as I wasn't sure how to get them to work with doctests. (hope to put them back eventually.)
ded1039added some sphinx_plot calls
f93aaf3Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__
c9b1aa4fixed a typo
f7394a8Implemented new method for sampling colors from the spectrum for multi-function plotting.
eb224abMerge 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 15 months 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 15 months ago by git

  • Commit changed from eb224ab0fa489d2444d179a4d78574559b52ec97 to fe05d0ab2a577acc0f2b93cb5d7e92989040cc81

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

fe05d0azapped the TAB characters

comment:25 Changed 15 months 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: Changed 15 months 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: Changed 15 months 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 15 months ago by paulmasson

  • Cc novoselt added

comment:29 Changed 15 months 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 15 months 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 15 months 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 15 months 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:

d169dc1Started adding multiple directives for color and linestyle to _plot()
58f8323multi-function plotting now allows for and to be dictionaries/lists/tuples. Similar functionality added for and .
2fb9172deleted superfluous whitespace
d02567emade plot-curve colors uniform, for ease of deciphering in lieu of a legend, when plot() is called with multiple functions.
c6ed638removed sphinx_plot calls, as I wasn't sure how to get them to work with doctests. (hope to put them back eventually.)
ded1039added some sphinx_plot calls
f93aaf3Merge branch 'develop' into t/12962/implement_multi_function_plotting_options_in_plot__
c9b1aa4fixed a typo
f7394a8Implemented new method for sampling colors from the spectrum for multi-function plotting.
276f8ebMerge 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 15 months 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 15 months 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 15 months ago by git

  • Commit changed from 276f8eb2f08a898e6be8b20c09dd9c5b4f49a5b0 to 02f19a24c59dd2a95801282725909f63924c3340

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

02f19a2Made default color choices when plot() is passed a list of functions.

comment:36 Changed 15 months 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 15 months 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 15 months 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 14 months ago by git

  • Commit changed from 02f19a24c59dd2a95801282725909f63924c3340 to 590325e590189723fb38174a159afddc223e37fc

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

590325edoctests pass; ValueError check works if color and rgbcolor are both passed; errors remain in building documentation

comment:40 follow-up: Changed 14 months 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 14 months ago by git

  • Commit changed from 590325e590189723fb38174a159afddc223e37fc to c836728856ed1c419f7bd53fa28e2ac57a941713

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

7d593cdremoved whitespace around keyword calls
c836728working 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: Changed 14 months 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 14 months ago by git

  • Commit changed from c836728856ed1c419f7bd53fa28e2ac57a941713 to dc1561813b407f70c433eeb0bacd6f7d6e5721ac

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

dc15618documentation now builds. problem was attempting to set frame and axes options in graphics_array with sphinx_plot.

comment:44 Changed 14 months 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 14 months ago by git

  • Commit changed from dc1561813b407f70c433eeb0bacd6f7d6e5721ac to 165dda5ed454d0f684a937eb997c08e82d47d6ab

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

165dda5cleaned up a few comments; deleted others no longer needed

comment:46 Changed 14 months ago by alauve

  • Status changed from needs_work to needs_review

comment:47 Changed 14 months ago by tscrim

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

comment:48 Changed 14 months ago by kcrisman

See e.g. #10657 and #11160.

comment:49 Changed 14 months 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: Changed 14 months 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 14 months 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: Changed 14 months 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 14 months 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 14 months ago by alauve

  • Status changed from needs_review to needs_work

comment:55 Changed 14 months ago by git

  • Commit changed from 165dda5ed454d0f684a937eb997c08e82d47d6ab to 26ac1d3ef790319e5cedce5a3cd6ddb1242e943b

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

26ac1d3added a few more sphinx_plots; better described how default colors are chosen

comment:56 Changed 14 months ago by alauve

  • Status changed from needs_work to needs_review

comment:57 follow-up: Changed 14 months 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 14 months 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 14 months ago by paulmasson (previous) (diff)

comment:59 in reply to: ↑ 57 Changed 14 months 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 14 months ago by git

  • Commit changed from 26ac1d3ef790319e5cedce5a3cd6ddb1242e943b to 742422e65b2bc8321bb784bd5229ce12c08873dd

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

742422eModified behavior of fillcolor and legend_label, and reworked some of the sphinx_plot examples as well.

comment:61 Changed 14 months 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 14 months ago by vbraun

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

Branch seems to be missing?

comment:63 Changed 14 months ago by git

  • Commit set to 4acb9d65b9da7f60fb9e75c44344451e2b3f29ad

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

276f8ebMerge branch 'u/alauve/implement_multi_function_plotting_options_in_plot__' of trac.sagemath.org:sage into public/graphics/multi_function_plot_options-12962
02f19a2Made default color choices when plot() is passed a list of functions.
590325edoctests pass; ValueError check works if color and rgbcolor are both passed; errors remain in building documentation
7d593cdremoved whitespace around keyword calls
c836728working toward request to use rgbcolor instead of color. Simply making this change in L2139 breaks the plot()
dc15618documentation now builds. problem was attempting to set frame and axes options in graphics_array with sphinx_plot.
165dda5cleaned up a few comments; deleted others no longer needed
26ac1d3added a few more sphinx_plots; better described how default colors are chosen
742422eModified behavior of fillcolor and legend_label, and reworked some of the sphinx_plot examples as well.
4acb9d6Merge branch 'public/graphics/multi_function_plot_options-12962' of trac.sagemath.org:sage into public/graphics/multi_function_plot_options-12962

comment:64 Changed 14 months 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 14 months 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.