#19953 closed enhancement (fixed)
Add pictures to plot.py
Reported by:  jhonrubia6  Owned by:  

Priority:  minor  Milestone:  sage7.1 
Component:  documentation  Keywords:  plot, docstring 
Cc:  jmantysalo, egourgoulhon, kcrisman  Merged in:  
Authors:  Javier Honrubia González  Reviewers:  Eric Gourgoulhon, John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  3a999e9 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Modify the documentation of plot.py to incorporate pictures of the examples which produce plot
Change History (28)
comment:1 Changed 7 years ago by
Authors:  → Javier Honrubia Gonzalez 

Component:  PLEASE CHANGE → documentation 
Description:  modified (diff) 
Keywords:  plot docstring added 
Priority:  major → minor 
Type:  PLEASE CHANGE → enhancement 
comment:2 Changed 7 years ago by
Cc:  jmantysalo added 

comment:3 Changed 7 years ago by
Cc:  egourgoulhon added 

comment:4 Changed 7 years ago by
Authors:  Javier Honrubia Gonzalez 

Cc:  kcrisman added 
comment:5 Changed 7 years ago by
comment:6 Changed 7 years ago by
There is at least #18471 for this thing; please mark it as duplicate when you are done.
comment:7 Changed 7 years ago by
Branch:  → u/jhonrubia6/add_pictures_to_plot_py 

comment:8 Changed 7 years ago by
Authors:  → Javier Honrubia Gonzalez 

Commit:  → 6ed2d32c6bb49cf93156bbc2ce10323a1decbabf 
New commits:
6ed2d32  Partial commit with 85 graphics

comment:9 Changed 7 years ago by
I included some 87 graphics in this commit. I would appreciate your comments. It is beginning to be a slow process, rewarding for me, but I ignore if we want to pay the price when building the reference documentation. A compromise solution would be not to generate all the graphics which have the comment #long time (good hint!), in addition to those figures which cannot be built since the implied functions do not have a plot() function.
comment:11 followup: 12 Changed 7 years ago by
I have only been paying a little attention to this, but I would say that it might be better to have an on/off switch so that many developers don't have to waste time waiting for this. But so that the online documentation has full pictures. ?
comment:12 Changed 7 years ago by
Replying to kcrisman:
I have only been paying a little attention to this, but I would say that it might be better to have an on/off switch so that many developers don't have to waste time waiting for this. But so that the online documentation has full pictures. ?
There is an option
sage docbuild noplot
documented as "do not include graphics autogenerated using the 'plot' markup". I just tested it and it works.
comment:13 followup: 15 Changed 7 years ago by
That sounds familiar. So then the question is which one should be default? I think you have already linked to the discussion about that somewhere.
PS there is at least one typo:
 sage: plot(x**2, (x,0,3), ticks=[[1,2.5],pi/2], tick_formatter=[["$x_1$","$x_2$"],pi]) # long time + sage: plot(x2, (x,0,3), ticks=[[1,2.5],pi/2], tick_formatter=[["$x_1$","$x_2$"],pi]) # long time
will definitely raise an error about x2
.
comment:14 Changed 7 years ago by
Commit:  6ed2d32c6bb49cf93156bbc2ce10323a1decbabf → a35837d720ebb545cd2f62d10741a7a75c437772 

Branch pushed to git repo; I updated commit sha1. New commits:
a35837d  Added all the 'drawable' pictures in plot.py

comment:15 Changed 7 years ago by
Description:  modified (diff) 

Status:  new → needs_review 
Replying to kcrisman:
That sounds familiar. So then the question is which one should be default? I think you have already linked to the discussion about that somewhere.
PS there is at least one typo:
 sage: plot(x**2, (x,0,3), ticks=[[1,2.5],pi/2], tick_formatter=[["$x_1$","$x_2$"],pi]) # long time + sage: plot(x2, (x,0,3), ticks=[[1,2.5],pi/2], tick_formatter=[["$x_1$","$x_2$"],pi]) # long timewill definitely raise an error about
x2
.
typo corrected, thanx
comment:16 followup: 18 Changed 7 years ago by
Reviewers:  → Eric Gourgoulhon 

Status:  needs_review → needs_work 
This is very nice; thank you for this work!
I've noticed some doctest error though: with sage 7.1.beta3,
./sage t long src/sage/plot/plot.py
results in
Error: TAB character found at line 1518
Indeed there are 2 tab characters at that line.
Moreover, there are spurious blank spaces in that file. You should suppress them by telling your editor to remove trailing whitespaces.
comment:17 Changed 7 years ago by
Commit:  a35837d720ebb545cd2f62d10741a7a75c437772 → 42b9e4ccb4cf7a5cc6c2f93f304359d34059e23d 

Branch pushed to git repo; I updated commit sha1. New commits:
42b9e4c  Removed trailing blanks and spurious tabs

comment:18 Changed 7 years ago by
Replying to egourgoulhon: Thank you for your feedback
I've noticed some doctest error though: with sage 7.1.beta3,
./sage t long src/sage/plot/plot.py
results inError: TAB character found at line 1518
Indeed there are 2 tab characters at that line.
not any more :)
Moreover, there are spurious blank spaces in that file. You should suppress them by telling your editor to remove trailing whitespaces.
also gone.
comment:19 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:20 Changed 7 years ago by
Status:  needs_review → positive_review 

Thanks for the changes. Everything seems fine. I've also checked that the pdf doc produced with ./sage docbuild reference/plotting pdf
is correctly generated, with all the figures included.
Regarding the extra CPU time discussed in comment:9 to comment:15, it took only 1 min 30 s to generate all the html plot section on my computer, which seems acceptable, given the huge benefit of having nice figures in the doc. So I suggest to leave the picture generation as the default and I am setting a positive review. Thanks again for this work!
comment:21 followup: 23 Changed 7 years ago by
Status:  positive_review → needs_work 

I object to the change in plot/graphics.py
. Either add a real example or don't include an EXAMPLES
block. There should also be a newline before the start of the method.
comment:22 Changed 7 years ago by
Commit:  42b9e4ccb4cf7a5cc6c2f93f304359d34059e23d → 3a999e90a1fe6132e5ca0d2f903fe370ff5d8b14 

Branch pushed to git repo; I updated commit sha1. New commits:
3a999e9  Example added to the plot method in graphicsarray

comment:23 Changed 7 years ago by
Replying to jhpalmieri:
I object to the change in
plot/graphics.py
. Either add a real example or don't include anEXAMPLES
block. There should also be a newline before the start of the method.
you are right. I forgot to add the example, thank you for pointing it out. I think it is fixed now with an example.
comment:24 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:25 Changed 7 years ago by
Status:  needs_review → positive_review 

That looks good to me, thanks.
comment:26 Changed 7 years ago by
Reviewers:  Eric Gourgoulhon → Eric Gourgoulhon,John Palmieri 

comment:27 Changed 7 years ago by
Branch:  u/jhonrubia6/add_pictures_to_plot_py → 3a999e90a1fe6132e5ca0d2f903fe370ff5d8b14 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:28 Changed 7 years ago by
Authors:  Javier Honrubia Gonzalez → Javier Honrubia González 

Commit:  3a999e90a1fe6132e5ca0d2f903fe370ff5d8b14 
Reviewers:  Eric Gourgoulhon,John Palmieri → Eric Gourgoulhon, John Palmieri 
Since
What do you suggest? Do I create another ticket for that or just leave it in this ticket?