#19953 closed enhancement (fixed)
Add pictures to plot.py
Reported by: | jhonrubia6 | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-7.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 follow-up: 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 --no-plot
documented as "do not include graphics auto-generated using the 'plot' markup". I just tested it and it works.
comment:13 follow-up: 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 follow-up: 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 follow-up: 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?