Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#27085 closed enhancement (fixed)

documentation: Improve sphinx_plot to create scalable graphics

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-8.7
Component: documentation Keywords:
Cc: vbraun, vdelecroix, tmonteil, jmantysalo, ​fredrik.johansson, jdemeyer, jhpalmieri Merged in:
Authors: Matthias Koeppe Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 9daa32e (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

The function sphinx_plot, introduced in #17498 to support the .. PLOT:: directive provided by the matplotlib sphinx extension, creates all graphics by saving to a temporary PNG file first.

This ticket improves this function so that 2d graphics (including GraphicsArrays) are shipped out directly via matplotlib. In this way, proper SVG graphics can be generated for the HTML documentation.

Change History (23)

comment:1 Changed 3 years ago by mkoeppe

  • Branch set to public/sphinx_plot_svg

comment:2 Changed 3 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 02c559079c1b480f8446bdf805e400a689028372
  • Status changed from new to needs_review

New commits:

02c5590Improve sphinx_plot to create scalable graphics

comment:3 Changed 3 years ago by jhpalmieri

  • Cc jhpalmieri added

comment:4 Changed 3 years ago by jhpalmieri

The individual plots look good. It's a little quicker than the current way of building the documentation, and it takes up a little less disk space.

comment:5 Changed 3 years ago by jhpalmieri

I think this looks okay, but I would like more opinions.

comment:6 follow-up: Changed 3 years ago by kdilks

I checked the once case where I remember using a Sphinx plot in my documentation (Fully Packed Loops), and it works fine there. Is there a good place to check to see how the GraphicsArray? works?

comment:7 in reply to: ↑ 6 Changed 3 years ago by mkoeppe

Replying to kdilks:

I checked the once case where I remember using a Sphinx plot in my documentation (Fully Packed Loops), and it works fine there. Is there a good place to check to see how the GraphicsArray? works?

The documentation for sage.plot.plot.graphics_array has an example plot.

comment:8 Changed 3 years ago by kdilks

Checked those, everything looks good to me. The svg files are taking up marginally more space than the original png files for me, but not enough to be concerning (71 vs 63 kb for plot-24, 45 vs 34 kb for plot-25 in the sage.plot.plot.graphics_array documentation).

comment:9 Changed 3 years ago by jhpalmieri

The advantage, disk-space wise, is I see a bunch of PDF files in the old version but smaller svg files in the new version. I also see two versions of png files in the old version and only one in the new.

comment:10 Changed 3 years ago by mkoeppe

I have only tested generating HTML documentation. I don't know what other formats are expected to work. In particular I haven't tried to build the PDF documentation.

comment:11 Changed 3 years ago by jhpalmieri

With vanilla Sage, I get PDF files in local/share/doc/sage/inventory/en/reference/plotting/sage/graphs/ when I'm building the html docs. For example:

  -rw-r--r--  1 palmieri staff 254903 Jan 24 19:51 graph_plot-1.hires.png
  -rw-r--r--  1 palmieri staff 215503 Jan 24 19:51 graph_plot-1.pdf
  -rw-r--r--  1 palmieri staff  60166 Jan 24 19:51 graph_plot-1.png
  -rw-r--r--  1 palmieri staff     34 Jan 24 19:51 graph_plot-1.py
  ...

With the branch here:

  -rw-r--r--  1 palmieri staff  38735 Jan 24 20:03 graph_plot-1.png
  -rw-r--r--  1 palmieri staff     34 Jan 24 20:03 graph_plot-1.py
  -rw-r--r--  1 palmieri staff  18445 Jan 24 20:03 graph_plot-1.svg

comment:12 Changed 3 years ago by kdilks

I was also only checking the HTML documentation, and in a not-so-thorough way. I'll look a little more closely to double check on all the files that get created, and also try building the pdf documentation.

comment:13 Changed 3 years ago by git

  • Commit changed from 02c559079c1b480f8446bdf805e400a689028372 to 9daa32e2933d7dabdfed8b2284d544a7b5f77552

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

9daa32eFixup for scalable graphics in PDF docs

comment:14 Changed 3 years ago by mkoeppe

With this fix, also the PDF documentation now has the scalable graphics. Needs review...

comment:15 follow-up: Changed 3 years ago by jhpalmieri

Regarding the change

-def sphinx_plot(plot):
+def sphinx_plot(graphics, **kwds)

is sphinx_plot called with extra arguments anywhere (yet)?

It all works for me, but I don't have time to look carefully at the rest of the code. At a glance it looks okay, but if someone else is willing to take a closer look, that would be great.

comment:16 in reply to: ↑ 15 Changed 3 years ago by mkoeppe

Replying to jhpalmieri:

Regarding the change

-def sphinx_plot(plot):
+def sphinx_plot(graphics, **kwds)

is sphinx_plot called with extra arguments anywhere (yet)?

It is not yet used in the sage documentation, but is crucial for GraphicsArrays, which do not have a way to store show options. (One of many defects of GraphicsArrays.) I use it in my package to make graphics array plots with particular figure sizes. Example: http://mkoeppe.github.io/cutgeneratingfunctionology/doc/html/procedures.html

comment:17 follow-up: Changed 3 years ago by jhpalmieri

At some point (not necessarily on this ticket), would it be a good idea to move sphinx_plot into the Sage library, along with a docstring, examples, and doctests, and then essentially just import it in conf.py? So change plot_pre_code to equal

from sage.all_cmdline import *
from sage.plot.sphinx_plot import sphinx_plot

Or is there some good reason to keep it where it is?

comment:18 in reply to: ↑ 17 ; follow-up: Changed 3 years ago by mkoeppe

Replying to jhpalmieri:

At some point (not necessarily on this ticket), would it be a good idea to move sphinx_plot into the Sage library, along with a docstring, examples, and doctests, and then essentially just import it in conf.py? So change plot_pre_code to equal

from sage.all_cmdline import *
from sage.plot.sphinx_plot import sphinx_plot

Or is there some good reason to keep it where it is?

In a follow-up ticket, it could be good to refactor it as follows: Remove the sphinx_plot function completely and instead giving every graphics object (including GraphicsArrays and the 3D graphics objects) a method to_pyplot (or similar) that will put the object in a pyplot figure.

This would be useful for interoperating sage graphics with pure matplotlib.pyplot code, not just for Sphinx.

comment:19 Changed 3 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Okay, I'm happy with this.

comment:20 Changed 3 years ago by mkoeppe

Thank you!

comment:21 in reply to: ↑ 18 Changed 3 years ago by mkoeppe

Replying to mkoeppe:

In a follow-up ticket, it could be good to refactor it as follows: Remove the sphinx_plot function completely and instead giving every graphics object (including GraphicsArrays and the 3D graphics objects) a method to_pyplot (or similar) that will put the object in a pyplot figure.

This would be useful for interoperating sage graphics with pure matplotlib.pyplot code, not just for Sphinx.

Follow-up ticket for this: #27276

comment:22 Changed 3 years ago by vbraun

  • Branch changed from public/sphinx_plot_svg to 9daa32e2933d7dabdfed8b2284d544a7b5f77552
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:23 Changed 3 years ago by mkoeppe

  • Commit 9daa32e2933d7dabdfed8b2284d544a7b5f77552 deleted

Follow-up: #27481

Note: See TracTickets for help on using tickets.