Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong cross-referencing in modindex of documentation #21044

Closed
haraldschilly opened this issue Jul 18, 2016 · 26 comments
Closed

wrong cross-referencing in modindex of documentation #21044

haraldschilly opened this issue Jul 18, 2016 · 26 comments

Comments

@haraldschilly
Copy link
Member

The Sphinx-generated documentation has index files for modules. E.g. looking at http://doc.sagemath.org/html/en/a_tour_of_sage/py-modindex.html

Checking upon the first link in this list, it points to a non-existing page. It seems like as if sphinx doesn't correctly cross link to a sibling. I.e. instead of /html/en/a_tour_of_sage/...

http://doc.sagemath.org/html/en/a_tour_of_sage/algebras/sage/algebras/affine_nil_temperley_lieb.html#module-sage.algebras.affine_nil_temperley_lieb

it should be /html/en/reference/...

http://doc.sagemath.org/html/en/reference/algebras/sage/algebras/affine_nil_temperley_lieb.html#module-sage.algebras.affine_nil_temperley_lieb

Upstream (Sphinx) fix:

Upstream: Fixed upstream, in a later stable release.

CC: @paulmasson @embray @jdemeyer @hivert @egourgoulhon @slel

Component: documentation

Author: Erik Bray

Branch: dbfb2d6

Reviewer: Paul Masson

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

@embray
Copy link
Contributor

embray commented Jul 27, 2016

comment:3

As I found on the mailing list, I encountered this problem in the doctests, with an incorrect link being generated for sage.rings.padics.tutorial.

This behavior can be explained in part by this comment in the sphinx.ext.intersphinx source:

        # Duplicate values in different inventories will shadow each
        # other; which one will override which can vary between builds
        # since they are specified using an unordered dict.  To make
        # it more consistent, we sort the named inventories and then
        # add the unnamed inventories last.  This means that the
        # unnamed inventories will shadow the named ones but the named
        # ones can still be accessed when the name is specified.

In my specific case, entries for sage.rings.padics.tutorial are showing up in the objects.inv for each of

/home/embray/src/sagemath/sage-cygwin/local/share/doc/sage/html/en/reference/plotting
/home/embray/src/sagemath/sage-cygwin/local/share/doc/sage/html/en/reference/padics
/home/embray/src/sagemath/sage-cygwin/local/share/doc/sage/html/en/reference/plot3d	

Since these are stored as keys in a dict the order can nondeterministic. In my case plot3d ends up being last.

What's still mysterious is why sage.rings.padics.tutorials would even have reference in the plotting or plot3d docs' objects.inv. Neither of those documents even reference it.

@embray
Copy link
Contributor

embray commented Jul 27, 2016

comment:4

Incidentally when I run my serial docs build, plot3d and plotting are built immediately after padics.

If I understand the build system right, however, each sub-document should be built in its own sphinx environment. Yet it seems as if there's some "leaking" between the environments during the initial pass, and this shouldn't happen.

@haraldschilly
Copy link
Member Author

comment:5

"leaking" between the environments ...

oh, good point! that could explain why those wrong links show up at all. my wild guess: sphinx generates "temporary" files to cache some data-structures. on the next pass, sphinx doesn't clean up but somehow loads and reuses these caches. Maybe it's already enough to use find -cmin -5 or so to check recently modified files to pinpoint if/where such suspicious cache files are?

@embray
Copy link
Contributor

embray commented Jul 28, 2016

comment:6

It's also resulting in huge objects.inv inventories for relatively small documents, because all these labels and references are being carried from one environment to the next.

It could also be the reason for some other issues I've had (for which it seems I haven't posted a ticket) with duplicate labels between some documents. The duplicate labels really shouldn't matter as long as they're not within the same document.

@embray
Copy link
Contributor

embray commented Jul 28, 2016

comment:7

Ah, I've found it. This is a bug in Sphinx. A nasty case of "dict.copy() is a shallow copy"...

@embray
Copy link
Contributor

embray commented Jul 28, 2016

Upstream: Reported upstream. No feedback yet.

@embray
Copy link
Contributor

embray commented Jul 28, 2016

comment:8

Reported upstream: sphinx-doc/sphinx#2816

I have a fix in the sage end (via monkey patching) that I'm testing now.

@embray
Copy link
Contributor

embray commented Aug 19, 2016

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

@embray
Copy link
Contributor

embray commented Aug 19, 2016

comment:10

Here's a workaround for this bug in the meantime. It's ugly, but that's to be expected. I've had this patch in my working branch for some time now and it has been well-tested.


New commits:

dbfb2d6Monkey-patch for doc build issue caused by bug in Sphinx

@embray
Copy link
Contributor

embray commented Aug 19, 2016

Commit: dbfb2d6

@embray
Copy link
Contributor

embray commented Aug 19, 2016

Author: Erik Bray

@embray
Copy link
Contributor

embray commented Aug 19, 2016

Branch: u/embray/patch-domain-init

@embray
Copy link
Contributor

embray commented Aug 19, 2016

comment:11

Also a nice side-effect is that it results in smaller objects.inv files, which were previously full of unnecessary duplicates.

@embray
Copy link
Contributor

embray commented Aug 24, 2016

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

@embray
Copy link
Contributor

embray commented Aug 24, 2016

comment:12

The fix will be upstream in Sphinx 1.4.6, though that doesn't do us any immediate good since we're a ways from being able to upgrade Sphinx IIUC.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Sep 22, 2016

comment:14

After building Sage and the documentation with this branch, I randomly tested links in py-modindex.html files and didn't find any bad links. Needless to say, there is now no such file in either html/en/a_tour_of_sage as reported above, or in html/en/thematic_tutorials as currently appears in Google's webmaster tools.

I don't know enough about this part of Sage to comment on whether this is the best way to fix the problem, but it certainly does appear to deal with the issue. This should really get into 7.4 so that the next version of the documentation will look better to Google. What else needs to be tested for that to happen?

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Sep 22, 2016

Reviewer: Paul Masson

@paulmasson paulmasson mannequin modified the milestones: sage-7.3, sage-7.4 Sep 22, 2016
@embray
Copy link
Contributor

embray commented Sep 22, 2016

comment:16

In addition to fixing the specific issue in the ticket description, this fixes many other problems with building the docs in a single process that arose due to labels being shared between documents, resulting in "duplicate label" errors and similar.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Sep 23, 2016

comment:17

If there are no objections from other potential reviewers, setting this to positive review.

@vbraun
Copy link
Member

vbraun commented Oct 3, 2016

Changed branch from u/embray/patch-domain-init to dbfb2d6

@vbraun vbraun closed this as completed in 9a9a868 Oct 3, 2016
@jdemeyer
Copy link

comment:19

Replying to @embray:

that doesn't do us any immediate good since we're a ways from being able to upgrade Sphinx IIUC.

Why not? Did you actually try it?

@jdemeyer
Copy link

Changed commit from dbfb2d6 to none

@embray
Copy link
Contributor

embray commented Nov 7, 2016

comment:20

I don't know why I wrote that 3 months ago. Last I recall I wasn't sure who was working on what w.r.t. Sphinx support or what the progress was on that.

@jdemeyer
Copy link

comment:21

Note: I will revert this in #22252.

@embray
Copy link
Contributor

embray commented Mar 14, 2017

comment:22

Sounds good--thanks!

@slel

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants