sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.3k stars 449 forks source link

include pictures in the reference manual and notebook introspection #6685

Closed jhpalmieri closed 15 years ago

jhpalmieri commented 15 years ago

Including pictures in the documentation is easy: a new "pictures" directory in doc/en/reference and a quick patch to MANIFEST.in to make sure the contents of that directory are included in distributions. Then in a docstring, a line like

.. image:: /pictures/sine.png

will include that file when the reference manual is built.

Getting those pictures included in docstrings in the notebook is trickier.

See sage-devel for some discussion.

CC: @qed777

Component: documentation

Author: John Palmieri, Mitesh Patel

Reviewer: Dan Drake

Merged: Sage 4.1.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/6685

jhpalmieri commented 15 years ago
comment:1

Here's a patch. It puts a picture in sage/plot/plot.py which you can see in the reference manual, and with a bit of work, in notebook introspection: type sage.plot.plot?. The work is: you need to make a symlink from SAGE_ROOT/doc/en/reference/pictures to SAGE_ROOT/doc/output/html/en/pictures.

Things still to do:

jhpalmieri commented 15 years ago
comment:2

Okay, the patch now creates the symlink. I say we don't worry about modifying detex right now; once we get this to work, we can think about that issue.

jhpalmieri commented 15 years ago

Attachment: trac_6685-first-draft.patch.gz

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago

Attempt to avoid symbolic links. Apply only this patch.

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago
comment:3

Attachment: trac_6685-ref_pictures_no_symlinks.patch.gz

In an attempt to avoid potential problems with symlinks (cf. #5799, #6614), I've made some changes and attached a patch that

I wish Sphinx could already generate collapsible divs for a docstring's various sections (EXAMPLES, TESTS, etc.).

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago
comment:4

A note to prospective reviewers and testers: Upgrade to Sphinx v0.6.x (cf. #6586) to use absolute image paths.

jhpalmieri commented 15 years ago
comment:5

Looks good to me. I have no opinion about "pictures" vs. "media". Should we change the link in plot.py to ../../pictures/plot/cos_and_triangle.png, so that it works with Sphinx 0.5.1?

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago

Should work with Sphinx 0.5.1. Changed 'pictures' to 'media'. Apply only this patch.

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago
comment:6

Attachment: trac_6685-ref_pictures_no_symlinks_v2.patch.gz

Done. Auto-generating and adding all 2D images under sage/plot/ seems to be less straightforward.

dandrake commented 15 years ago
comment:7

I'm reviewing this patch and things look very good so far. I do have a couple comments:

First, I like "media" for the directory, since we can now do video very easily in browsers like Firefox 3.5, so one can imagine having animations and so on in the reference manual.

Second, I'm not sure how I feel about including the binary file in the patch, and having hg track binary files. I'd rather have a nice way of specifying a way to produce the image. In the example you have in the reference manual, there's a sequence of Sage commands whose purpose is to produce the image we want to show. So can't we...use those commands to produce the image?

Perhaps the goal of this ticket should just be to add support for including images in the documentation and we can worry about nice ways to produce those images later.

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago
comment:8

By the way, and for what it's worth, #6694 was first motivated by the desire to interactively produce images for the reference manual. It is a fun way to explore the many mathematical capabilities of Sage, although evaluating all cells can sometimes severely test a browser.

But adapting the docbuild and/or doctesting frameworks seems to be a significantly better approach for generating the images in bulk. Does sage -t, when it's not invoked with -randorder, keep track of the index of a test cell in a module? If so, perhaps we can retain any produced images, rename them in a systematic way, and splice appropriate .. image:: tags into the docstrings. Since this is invasive, we can make this a separate command, e.g., sage -imagebuild, for library authors. Are regular expressions the safest way to check for and remove or update existing tags?

An alternative, possibly cleaner way is to extend Sphinx so that it generates and inserts an image dynamically, whenever it finds an .. autoimage:: directive. It may be simplest to include as a "required option" the precise code to make the image. This will increase the time it takes to build the manual, unless we cache the figures effectively. (Sphinx's doctest extension may or may not help here.)

Some library authors may wish to include a specific set of pictures in the reference manual, in part to showcase their code's abilities. If a media file is not superfluous, but it takes a long time to generate with Sage, why not include and track it as a binary? We could put some limits on file sizes or even offer to host them on sage.math and embed them in the manual.

My chief worry is maintaining the correctness and consistency of any pictures with the code examples. Adding image diffs to doctesting (with some tolerance) might help, but we would still need to track the binary "masters." Anyway, this might force contributors to pay attention.

dandrake commented 15 years ago
comment:9

Replying to @qed777:

Some library authors may wish to include a specific set of pictures in the reference manual, in part to showcase their code's abilities. If a media file is not superfluous, but it takes a long time to generate with Sage, why not include and track it as a binary? We could put some limits on file sizes or even offer to host them on sage.math and embed them in the manual.

I'm not terribly opposed to tracking binaries, so in some situation it's the best thing to do, we should do it. The "image doctest" idea is one place where we obviously should do this.

My chief worry is maintaining the correctness and consistency of any pictures with the code examples. Adding image diffs to doctesting (with some tolerance) might help, but we would still need to track the binary "masters." Anyway, this might force contributors to pay attention.

Maintaining correctness and consistency is a major reason why I'd prefer to have autogenerated stuff. If I've learned anything from the Sage doctesting philosophy, it's that you can never rely on people to "just notice" bugs and problems. The only thing people notice, it seems, is "X failures in..." at the end of a make test run!

jhpalmieri commented 15 years ago
comment:10

I would note that we already include some binaries: there are pictures in the directory SAGE_ROOT/sage/doc/en/bordeaux_2008 (modpcurve.png and birch.png).

I don't have any strong feelings about the particular picture included in the patch here; it was included as a test case, and I didn't put a lot of thought into finding a place in the reference manual where a picture would help but where that picture couldn't be easily (or quickly) autogenerated. I certainly have in mind some applications -- triangulations of various topological spaces, to be used in the simplicial complexes code -- where I want to include pictures that can not be produced by Sage.

Would it be better if I tried to find a more natural candidate for a picture? Or (as ddrake suggested), get rid of the binary and just have a way to include pictures for now? Then we can have a discussion on sage-devel, or wherever is appropriate, about just how many pictures (and other media) to include, and what criteria to use.

The idea of remotely hosting media is interesting, but I don't see a way to embed a remotely hosted image in reST: it looks like the image directive needs a path, not a URL. (I know how to do it in html, certainly, but is there Sphinx/reST code which would let us pass a URL to an image directive?)

dandrake commented 15 years ago

Attachment: trac_6685-proposal.patch.gz

Like the "v2" patch, but just adds support for including images

dandrake commented 15 years ago

Author: John Palmieri, Mitesh Patel

dandrake commented 15 years ago

Reviewer: Dan Drake

dandrake commented 15 years ago
comment:11

There's two things going on here: one, the code that allows us to use the ".. image::" directives and have them work; two, the parts of the patch that actually change some docstrings to include some images. I'm giving this a positive review, and have split the "v2" patch into two patches:

My positive review certainly applies to the "proposal" patch and I think the release manager should merge it.

I propose that we not merge the usage examples patch and just leave it here so people can see how the image inclusion stuff works. We can hash out elsewhere the details of how to autogenerate images, do image doctests, and so on. Thoughts?

dandrake commented 15 years ago
comment:12

Attachment: trac_6685-usage-examples.patch.gz

Oops, I didn't see this before.

Replying to @jhpalmieri:

I don't have any strong feelings about the particular picture included in the patch here; it was included as a test case, and I didn't put a lot of thought into finding a place in the reference manual where a picture would help but where that picture couldn't be easily (or quickly) autogenerated. I certainly have in mind some applications -- triangulations of various topological spaces, to be used in the simplicial complexes code -- where I want to include pictures that can not be produced by Sage.

I like this idea, since if Sage can't produce the graphic, we need to track the image file. How about you find one of your applications and put up a patch here (to be applied on top of the "proposal" patch), and then we can ignore the usage-examples patch?

The idea of remotely hosting media is interesting, but I don't see a way to embed a remotely hosted image in reST: it looks like the image directive needs a path, not a URL. (I know how to do it in html, certainly, but is there Sphinx/reST code which would let us pass a URL to an image directive?)

Remote hosting sounds nice, but I'd like to keep the documentation self-contained, so that one can have everything available while, say, working on a laptop on an airplane. Although I wouldn't mind if a nonessential bit was linked, and something in the text said that the bit won't show up unless you have a network connection. Then one could include, say, a really big video file. Although all of this is moot if we can't figure out how to get the links into the docs! (I certainly don't know how.)

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 15 years ago

Merged: Sage 4.1.2.alpha0

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 15 years ago
comment:13

Merged trac_6685-proposal.patch.

While doctesting with the above patch, I had the following failure:

sage -t -long devel/sage-main/sage/misc/getusage.py
**********************************************************************
File "/scratch/mvngu/release/sage-4.1.1/devel/sage-main/sage/misc/getusage.py", line 69:
    sage: get_memory_usage(t)          # amount of memory more than when we defined t.
Expected:
    0.0
Got:
    0.34375
**********************************************************************
1 items had failures:
   1 of   4 in __main__.example_2
***Test Failed*** 1 failures.
For whitespace errors, see the file /scratch/mvngu/release/sage-4.1.1/tmp/.doctest_getusage.py
     [2.5 s]

This is the same failure as that reported in #6819. I'm merging patches and testing on sage.math at a time when it has about 3 GB of free RAM out of a total of 120 GB. As far as I can tell, the failure is not due to the above patch. One can doctest the file sage/misc/getusage.py with or without the patch and still occasionally gets that failure.

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago
comment:14

Replying to Dan Drake and John Palmieri:

Good points. Should we inquire about a contributed sage.math repository on sage-devel? If we cannot or prefer not to embed, we can include links to the supplemental material. To minimize the possibility of broken links, we could set up pages for different subject areas and point to them.

To embed almost anything, use the raw directive:

.. raw:: html

   <img src="http://www.sagemath.org/pix/sage_logo_new.png">

If this is too permissive, we could extend Sphinx with an embed directive, say.

e14f4152-4982-4ace-8c95-73a0599b109b commented 15 years ago
comment:15

I've opened #6847 for plot automatons.