Opened 9 years ago

Closed 8 years ago

#10620 closed enhancement (fixed)

Upgrade Sphinx to version 1.1.2

Reported by: davidm Owned by: mvngu
Priority: minor Milestone: sage-4.8
Component: packages: standard Keywords: sphinx upgrade spkg autodoc
Cc: jason, hivert Merged in: sage-4.8.alpha2
Authors: John Palmieri Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jhpalmieri)

Just made a spkg and a change to sage_autodoc in order to upgrade sphinx to the latest version. Spkg and patch attached.


New spkg:

Apply

Attachments (8)

sage_autodoc.patch (749 bytes) - added by davidm 9 years ago.
trac_10620-sphinx-p0.patch (3.8 KB) - added by jhpalmieri 8 years ago.
patch for sphinx pkg, for review only
trac_10620-sphinx-p1.patch (13.5 KB) - added by jhpalmieri 8 years ago.
patch for sphinx pkg, for review only
trac_10620-sage.patch (52.7 KB) - added by jhpalmieri 8 years ago.
trac_10620-sphinx-1.1.2.patch (10.1 KB) - added by jhpalmieri 8 years ago.
patch for sphinx 1.1.2 spkg; for review only
trac_10620-scripts-hgignore.patch (456 bytes) - added by jhpalmieri 8 years ago.
scripts repo
trac_10620-sageinspect.patch (1019 bytes) - added by jhpalmieri 8 years ago.
sage library repo
trac_10620-sagenb-sageinspect.patch (1.0 KB) - added by jhpalmieri 8 years ago.
sagenb repo

Download all attachments as: .zip

Change History (33)

Changed 9 years ago by davidm

comment:1 Changed 9 years ago by mhansen

In 4.6.1, Sphinx was upgraded to 1.0.4. Any new spkg should be based on the one found in #10350.

comment:2 Changed 9 years ago by mvngu

  • Milestone set to sage-4.6.2
  • Summary changed from Upgrade sphinx-0.9.6.p4 to sphinx-1.0.6 to Upgrade sphinx-1.0.4.p5 to sphinx-1.0.6

comment:3 Changed 9 years ago by jason

  • Cc jason added

comment:4 Changed 9 years ago by jason

  • Status changed from new to needs_review

Should this ticket be set to "needs review"?

I guess by now sphinx 1.0.7 is released.

comment:5 Changed 9 years ago by kini

We might want to wait for 1.1, depending on how long it will take, since 1.1 includes MathJax? (hurrah!).

comment:6 Changed 8 years ago by jhpalmieri

  • Status changed from needs_review to needs_work

The patch for sage_autodoc needs a lot more work: the file sage_autodoc should be kept in line with the file autodoc.py in sphinx/ext. Should we somehow autogenerate this file (using patch) from autodoc.py?

comment:7 Changed 8 years ago by jhpalmieri

In fact, maybe we should include sage_autodoc.py in the Sphinx spkg: make a copy of autodoc.py, patch it, and write it in the sphinx/ext directory. That would make it much easier to keep sage_autodoc.py in sync with autodoc.py. Then we just have to make this change in SAGE_ROOT/devel/sage/doc/common/conf.py:

  • doc/common/conf.py

    diff --git a/doc/common/conf.py b/doc/common/conf.py
    a b sys.path.append(os.path.abspath(os.path. 
    1313
    1414# Add any Sphinx extension module names here, as strings. They can be extensions
    1515# coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
    16 extensions = ['sage_autodoc',  'sphinx.ext.graphviz',
     16extensions = ['sphinx.ext.sage_autodoc',  'sphinx.ext.graphviz',
    1717              'sphinx.ext.inheritance_diagram', 'sphinx.ext.todo',
    1818              'sphinx.ext.intersphinx']
    1919# also of interest:

comment:8 Changed 8 years ago by jason

#10715 is also an "upgrade sphinx" ticket that might be considered a dup of this ticket.

comment:9 follow-up: Changed 8 years ago by leif

  • Keywords upgrade spkg autodoc added; uprgrade removed
  • Summary changed from Upgrade sphinx-1.0.4.p5 to sphinx-1.0.6 to Upgrade Sphinx to version 1.0.6 (or later)

And the spkg has to be rebased on that of #11659 (1.0.4.p7).

comment:10 in reply to: ↑ 9 Changed 8 years ago by leif

Replying to leif:

And the spkg has to be rebased on that of #11659 (1.0.4.p7).

Oh, just remembered I had prepared a follow-up Sphinx 1.0.4.p8 spkg at #11665 myself (currently needing review).

comment:11 Changed 8 years ago by jhpalmieri

  • Component changed from documentation to packages

Version 1.0.8 of Sphinx has just been released.

comment:12 Changed 8 years ago by jhpalmieri

  • Cc hivert added
  • Description modified (diff)

Okay, I have a new spkg, actually, two. The first ("p0") is just a straightforward upgrade of the previous version. However, we should probably update sage_autodoc.py in light of changes to the Sphinx extension autodoc.py, on which it is based. I can provide a patch to the Sage library modifying sage_autodoc.py if necessary.

A different approach: the second spkg ("p1") autogenerates sage_autodoc.py from autodoc.py, and saves sage_autodoc.py in sphinx/ext instead of in sage/doc/common. So using this one requires a change to sage/doc/common/conf.py, which I've provided. This approach seems as though it will be easier to maintain, or at least it will be easier to keep sage_autodoc.py in line with autodoc.py.

With both of these approaches, I get some warnings about cross-references, which are why the other entries are in the patch for the Sage library. I'm seeing lots of broken cross-references, though: take a look at the reference manual for categories/examples/finite_weyl_groups.py:

    Only the following basic operations are implemented:
     - :meth:`one`
     - :meth:`product`
     - :meth:`simple_reflection`
     - :meth:`Element.has_right_descent`.
{{{
None of these produces a link.  I don't know what's going on.
}}}

Changed 8 years ago by jhpalmieri

patch for sphinx pkg, for review only

Changed 8 years ago by jhpalmieri

patch for sphinx pkg, for review only

Changed 8 years ago by jhpalmieri

comment:13 follow-up: Changed 8 years ago by jhpalmieri

By the way, we could use the new spkg (either version) and keep the Sage library intact; this would use the old, and now somewhat out-dated, sage_autodoc.py, but it does not have the cross-reference problems.

comment:14 in reply to: ↑ 13 ; follow-up: Changed 8 years ago by hivert

Replying to jhpalmieri:

Thanks for building the new spkg !

By the way, we could use the new spkg (either version) and keep the Sage library intact; this would use the old, and now somewhat out-dated, sage_autodoc.py, but it does not have the cross-reference problems.

Concerning cross references, I've been doing some work in #9128. So my suggestion is to upgrade the spkg (I can probably find the time to review it this week) and the keep the cross reference problem for another ticket. What to you think ?

comment:15 in reply to: ↑ 14 Changed 8 years ago by jhpalmieri

  • Status changed from needs_work to needs_review

Replying to hivert:

Replying to jhpalmieri:

Thanks for building the new spkg !

By the way, we could use the new spkg (either version) and keep the Sage library intact; this would use the old, and now somewhat out-dated, sage_autodoc.py, but it does not have the cross-reference problems.

Concerning cross references, I've been doing some work in #9128. So my suggestion is to upgrade the spkg (I can probably find the time to review it this week) and the keep the cross reference problem for another ticket. What to you think ?

It makes me uncomfortable to have sage_autodoc.py so out-of-synch with autodoc.py, and as far as I can tell, changes to autodoc.py, and hence to the updated sage_autodoc.py, are what is causing the problem with the cross-references. So I think just merging the spkg would only be acceptable if the work on #9128 (or elsewhere?) includes updating sage_autodoc.py. What do you think of the idea of putting that file in the Sphinx directory (as in the "p1" version of the spkg) instead of keeping it in the Sage library?

comment:16 Changed 8 years ago by jhpalmieri

  • Authors changed from David Monarres to John Palmieri

comment:17 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Summary changed from Upgrade Sphinx to version 1.0.6 (or later) to Upgrade Sphinx to version 1.1.2

There is now a version 1.1.2 of Sphinx. Probably we should update this spkg.

comment:18 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I updated the "straightforward" spkg, I have no clue what autodoc is, so I left that alone.

comment:19 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:20 Changed 8 years ago by jhpalmieri

  • Description modified (diff)

sage_autodoc is the Sage version of the Sphinx extension autodoc (found in sphinx/ext/); it is used to extract the reference manual from the source code. We've modified the original to handle various things (perhaps cython code, lazy methods, cached methods?). So sage_autodoc.py, which lives in devel/sage/doc/common, used to be a simple modification of the Sphinx file autodoc.py, but now the two files differ more dramatically as autodoc.py has been patched in ways that sage_autodoc.py hasn't, and vice versa.

It might be good, to take full advantage of upgrades to Sphinx, to try to remedy this by creating sage_autodoc.py by patching autodoc.py in the Sphinx spkg. Unfortunately, when I tried this approach, the new sage_autodoc module didn't work perfectly, probably because of recent changes in autodoc. This could be dealt with on another ticket. Following Leif's general suggestion about this kind of thing, perhaps we should add a "To do" item to the "Special Build Instructions" section of SPKG.txt. I've prepared a new spkg (based on yours) with this change. The spkg also removes the patch to 'autodoc.py', since I don't think we use that anymore: we use 'sage_autodoc.py' instead.

comment:21 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work

The following should be added to the scripts .hgignore:

sphinx-apidoc

comment:22 follow-up: Changed 8 years ago by jhpalmieri

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I think this is okay now. It's a little more complicated than I thought. First, Sage does use autodoc, in the 'sphinxify' routine when doing introspection. So I restored the autodoc patch. Second, with the new spkg, I was getting doctest failures: one piece of output in sageinspect.py was wrong. I think that the new output is actually better: it's processing a docstring using sphinxify and coming up with nicer output. So I think it's okay to change the output for the doctest.

So there are three or four patches, one for the scripts repo (hgignore), one for the Sage library (doctest), and one for the Sphinx spkg. The patch for the Sage library should come with a corresponding patch for sagenb, but I don't know what to do about that. Submit a ticket for the flask notebook? I'll include one for the current sagenb repo, in case that's the right thing to do.

Changed 8 years ago by jhpalmieri

patch for sphinx 1.1.2 spkg; for review only

Changed 8 years ago by jhpalmieri

scripts repo

Changed 8 years ago by jhpalmieri

sage library repo

Changed 8 years ago by jhpalmieri

sagenb repo

comment:23 in reply to: ↑ 22 Changed 8 years ago by jdemeyer

Replying to jhpalmieri:

I'll include one for the current sagenb repo, in case that's the right thing to do.

Yes, this is the right thing to do (I have discussed with the sagenb developers some time ago).

comment:24 Changed 8 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

Looks good to me, but see the encoding issue at #10112.

comment:25 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.