Opened 4 years ago

Closed 3 years ago

Last modified 2 years ago

#21044 closed defect (fixed)

wrong cross-referencing in modindex of documentation

Reported by: schilly Owned by:
Priority: major Milestone: sage-7.4
Component: documentation Keywords:
Cc: paulmasson, embray, jdemeyer, hivert, egourgoulhon, slelievre Merged in:
Authors: Erik Bray Reviewers: Paul Masson
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: dbfb2d6 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by slelievre)

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:

Change History (23)

comment:1 Changed 4 years ago by paulmasson

  • Cc paulmasson added

comment:2 Changed 4 years ago by embray

  • Cc embray added

comment:3 Changed 4 years ago by embray

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.

comment:4 Changed 4 years ago by embray

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.

comment:5 Changed 4 years ago by schilly

"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?

comment:6 Changed 4 years ago by embray

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.

comment:7 Changed 4 years ago by embray

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

comment:8 Changed 4 years ago by embray

  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

Reported upstream: https://github.com/sphinx-doc/sphinx/pull/2816

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

comment:9 Changed 4 years ago by embray

  • Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.

comment:10 Changed 4 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/patch-domain-init
  • Commit set to dbfb2d6923ac29cb84f5d7cf927ac979ab3b0777
  • Status changed from new to needs_review

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

comment:11 Changed 4 years ago by embray

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

comment:12 follow-up: Changed 4 years ago by embray

  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

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.

comment:13 Changed 4 years ago by slelievre

  • Cc jdemeyer hivert egourgoulhon slelievre added

comment:14 Changed 4 years ago by paulmasson

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?

comment:15 Changed 4 years ago by paulmasson

  • Milestone changed from sage-7.3 to sage-7.4
  • Reviewers set to Paul Masson

comment:16 Changed 4 years ago by embray

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.

comment:17 Changed 4 years ago by paulmasson

  • Status changed from needs_review to positive_review

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

comment:18 Changed 3 years ago by vbraun

  • Branch changed from u/embray/patch-domain-init to dbfb2d6923ac29cb84f5d7cf927ac979ab3b0777
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:19 in reply to: ↑ 12 Changed 3 years ago by jdemeyer

  • Commit dbfb2d6923ac29cb84f5d7cf927ac979ab3b0777 deleted

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?

comment:20 Changed 3 years ago by embray

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.

comment:21 Changed 3 years ago by jdemeyer

Note: I will revert this in #22252.

comment:22 Changed 3 years ago by embray

Sounds good--thanks!

comment:23 Changed 2 years ago by slelievre

  • Description modified (diff)
Note: See TracTickets for help on using tickets.