Opened 2 years ago

Closed 19 months ago

#26451 closed enhancement (fixed)

Upgrade to Sphinx 1.8.5

Reported by: saraedum Owned by:
Priority: minor Milestone: sage-8.8
Component: packages: standard Keywords: sphinx, conda, upgrade
Cc: gh-timokau, fbissey, arojas, embray, slelievre, jhpalmieri, dimpase, thansen Merged in:
Authors: Julian Rüth, Jeroen Demeyer, John Palmieri, Tobias Hansen Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: f1a08e3 (Commits) Commit: f1a08e3522b66afe96e8fdd389cf1114b1db1365
Dependencies: #27528 Stopgaps:

Description (last modified by jhpalmieri)

This ticket upgrades Sphinx to version 1.8.5.

Our previous upgrade was to Sphinx 1.7.6 in #26033.

Tarball: https://files.pythonhosted.org/packages/2a/86/8e1e8400bb6eca5ed960917952600fce90599e1cb0d20ddedd81ba163370/Sphinx-1.8.5.tar.gz

Change History (137)

comment:1 Changed 2 years ago by saraedum

Note that there is nothing I can doctest here.

comment:2 Changed 2 years ago by saraedum

  • Branch set to u/saraedum/26451

comment:3 Changed 2 years ago by saraedum

  • Commit set to eb00e754ad464ed4e0374e86ffd0e779e6909369
  • Status changed from new to needs_review
  • Work issues set to is the patchbot happy?

New commits:

eb00e75Call Sphinx() 1.8 constructor

comment:4 Changed 2 years ago by saraedum

  • Cc gh-timokau fbissey arojas added

comment:5 Changed 2 years ago by saraedum

  • Keywords sphinx conda added

comment:6 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_info

I'd rather upgrade Sphinx while we're at it. That's a good thing to do and I would need to do that anyway to test this ticket.

comment:7 Changed 2 years ago by saraedum

  • Cc embray slelievre added

I was fearing that you would propose that ;)

I am not planning on working on upgrading Sphinx in Sage (as I've got too many other things to worry about.) I don't know what has chahnged in 1.8.1 and whether we would want to upgrade at all.

More generally, I don't know what we're aiming for here. I feel we don't have enough people to keep Sage-the-distribution close to upstream most of the time so that's probably not what we should be aiming for?

That said, feel free to upgrade Sphinx :) But if anybody likes these small changes and we want to move the Sphinx upgrade into a separate ticket, I would prefer that.

comment:8 Changed 2 years ago by saraedum

  • Description modified (diff)

comment:9 Changed 2 years ago by arojas

  • Description modified (diff)

Upgrading sphinx requires further changes to make docs build. I have a minimal patch at https://git.archlinux.org/svntogit/community.git/tree/trunk/sagemath-doc-sphinx-1.8.patch?h=packages/sagemath-doc to make it build, although with it one still gets lots of deprecation warnings due to changes in logging functions.

comment:10 Changed 2 years ago by arojas

  • Description modified (diff)

comment:11 follow-up: Changed 2 years ago by jdemeyer

Antonio, does Sphinx 1.8.1 work with just those changes (even if there are deprecation warnings)? If so, we might as well upgrade.

comment:12 in reply to: ↑ 11 Changed 2 years ago by arojas

Replying to jdemeyer:

Antonio, does Sphinx 1.8.1 work with just those changes (even if there are deprecation warnings)? If so, we might as well upgrade.

Yes, with Julian's patch and mine introspection works and docs build correctly.

comment:13 Changed 2 years ago by jdemeyer

  • Authors changed from Julian Rüth to Julian Rüth, Jeroen Demeyer
  • Component changed from documentation to packages: standard
  • Description modified (diff)
  • Summary changed from Prepare for sphinx 1.8 to Upgrade to sphinx 1.8.1
  • Work issues is the patchbot happy? deleted

comment:14 Changed 2 years ago by saraedum

jdemeyer, did you forget to push your branch? (you added yourself as an Author but did not push any changes it seems.)

comment:15 Changed 2 years ago by jdemeyer

Please give me time...

comment:16 Changed 2 years ago by saraedum

Note that there is a licensing issue in sphinxify.py, see #26453.

comment:17 Changed 2 years ago by git

  • Commit changed from eb00e754ad464ed4e0374e86ffd0e779e6909369 to a6051437fb79d3566313d641f1f31a79f2a8ff5e

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

a605143Call Sphinx 1.8.! with the correct outdir set

comment:18 Changed 2 years ago by git

  • Commit changed from a6051437fb79d3566313d641f1f31a79f2a8ff5e to 4ae155f09f45896ac8a06f35c128c500e3023deb

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4ae155fCall Sphinx 1.8.1 with the correct outdir set

comment:19 Changed 2 years ago by jdemeyer

  • Branch changed from u/saraedum/26451 to u/jdemeyer/26451

comment:20 Changed 2 years ago by jhpalmieri

  • Cc jhpalmieri added
  • Commit changed from 4ae155f09f45896ac8a06f35c128c500e3023deb to 19b29e552caf792bbd5eb1e335b6072f2125a981

New commits:

19b29e5Upgrade to Sphinx 1.8.1

comment:21 Changed 2 years ago by jhpalmieri

By the way, see also #26449 for issues with Sphinx and the Python 3 build of Sage.

comment:22 follow-up: Changed 2 years ago by gh-timokau

I just did the upgrade on nix and I also needed to apply the same patch to sagenb's sphinxify.py (https://github.com/sagemath/sagenb/pull/461).

comment:23 Changed 2 years ago by fbissey

  • Milestone changed from sage-8.4 to sage-8.5

comment:24 in reply to: ↑ 22 Changed 2 years ago by fbissey

Replying to gh-timokau:

I just did the upgrade on nix and I also needed to apply the same patch to sagenb's sphinxify.py (https://github.com/sagemath/sagenb/pull/461).

Once upon a time sphinxify was a part of sagenb and it was used in sage itself. So time ago we put an end to that. May be it is time to go in reverse and have (the now deprecated) sagenb use sage's sphinxify.

comment:25 Changed 2 years ago by jhpalmieri

The documentation does not build for me on OS X: I get

[reference]     valuations: 1 todos, 14 index, 1409 citations, 13 modules
[reference] ... done (472 todos, 2068 index, 1409 citations, 2019 modules)
[reference] preparing documents... skipping loading of indexes... done
[reference] Merging js index files...
[reference]     algebras: 3466 js index entries
[reference]     arithgroup: 966 js index entries

   ...

[reference]     structure: 1995 js index entries
[reference]     tensor_free_modules: 1035 js index entries
[reference]     valuations: 817 js index entries
[reference] ... done (44888 js index entries)
[reference] WARNING: cannot copy static file IOError(20, 'Not a directory')
[reference] copying static files... copying extra files... done
[reference] The HTML pages are in local/share/doc/sage/html/en/reference.
Error building the documentation.
Traceback (most recent call last):
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 1715, in main
    builder()
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 338, in _wrapper
    getattr(get_builder(document), name)(*args, **kwds)
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 536, in _wrapper
    getattr(DocBuilder(self.name, lang), format)(*args, **kwds)
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/__init__.py", line 130, in f
    runsphinx()
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 319, in runsphinx
    sys.stderr.raise_errors()
  File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage_setup/docbuild/sphinxbuild.py", line 254, in raise_errors
    raise OSError(self._error)
OSError: WARNING: cannot copy static file IOError(20, 'Not a directory')

Indeed, local/share/doc/sage/html/en/reference/_static is a file, not a directory: it is the file graphviz.css from Sphinx. If I delete it, create a directory _static, and run make again, then only graphviz.css is copied to the directory, so the reference manual is missing all of its css files. The other parts of the documentation look good.

Last edited 2 years ago by jhpalmieri (previous) (diff)

comment:26 Changed 2 years ago by jhpalmieri

There is some odd behavior with graphviz.css. First, it gets copied to local/share/doc/sage/inventory/en/reference/_static (a file, not a directory): Sphinx shouldn't copy any css files in the inventory build. Then it also gets copied to a similar location in the html build. I commented out 'sphinx.ext.graphviz' in the list of extensions in src/doc/common/conf.py and started over, but the same thing happened. So something strange is going on with graphviz and our custom builder for the reference manual. In contrast, ./sage --docbuild tutorial html works fine.

comment:28 in reply to: ↑ 27 Changed 2 years ago by jhpalmieri

Replying to jdemeyer:

See https://github.com/sphinx-doc/sphinx/pull/5549

That's very helpful, thank you, but it leads to my next problem: with that fix applied, graphviz.css is still copied to local/share/doc/sage/inventory/en/reference/_static/, and there is no reason to copy css files to the inventory build. As a consequence, I see this error:

Building reference manual, second pass.

[arithgrou] WARNING: failed to reach any of the inventories with the following issues:
[arithgrou] WARNING: intersphinx inventory '/Users/jpalmier/Desktop/Sage/git/sage/local/share/doc/sage/inventory/en/reference/_static/objects.inv' not fetchable due to <type 'exceptions.IOError'>: [Errno 2] No such file or directory: '/Users/jpalmier/Desktop/Sage/git/sage/local/share/doc/sage/inventory/en/reference/_static/objects.inv'

I think intersphinx looks in every subdirectory of inventory/en/reference/, including _static, which should not be there.

comment:29 Changed 2 years ago by jhpalmieri

And once I clean that up, I still see a file _static in the html reference build.

comment:30 Changed 2 years ago by jhpalmieri

I wonder if the problem is that it tries to copy graphviz.css to, for example, html/en/reference/algebras/_static, which is a symlink pointing to the as yet non-existent html/en/reference/_static.

comment:31 Changed 2 years ago by jhpalmieri

  • Branch changed from u/jdemeyer/26451 to u/jhpalmieri/26451

comment:32 Changed 2 years ago by jhpalmieri

  • Commit changed from 19b29e552caf792bbd5eb1e335b6072f2125a981 to 4a91a6005a550f99bd927b447d36cf7adc4e0f2c

Here is a new branch. I'm not happy about the changes to src/sage_setup/docbuild/__init__.py, but I couldn't get the reference manual to build otherwise. Maybe there is another Sphinx bug, or maybe it's something about our multidoc extension. I would be happy if someone found a cleaner fix.

(This requires Jeroen's Sphinx fix from https://github.com/sphinx-doc/sphinx/pull/5549, by the way. I did not add that as a patch to Sphinx, although maybe we will need to if Sphinx doesn't fix it themselves.)


New commits:

1efde59merging
b79b1d9trac 26451: silence deprecation warnings about app.info, app.debug, etc.
4a91a60trac 26451: remove bad _static directory, bad _static file

comment:33 Changed 2 years ago by git

  • Commit changed from 4a91a6005a550f99bd927b447d36cf7adc4e0f2c to 65ab61522cacad7558350f3cb1f0b16d0001235e

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

65ab615trac 26451: a little reST fix

comment:34 Changed 2 years ago by jhpalmieri

  • Dependencies set to #26558

With this (plus Jeroen's Sphinx fix, plus #26558 if your version of TeXLive is recent), the html and pdf docs build for me.

comment:35 Changed 2 years ago by slelievre

  • Description modified (diff)
  • Summary changed from Upgrade to sphinx 1.8.1 to Upgrade to Sphinx 1.8.2

The following tickets mentioned in the discussion above are now fixed and closed,

  • #26449 Sphinx and the Python 3 build of Sage
  • #26453 Relicense sphinxify.py
  • #26558 doc-pdf fails due to conflict with new babel.sty

The following pull request to Sphinx was merged on 2018-10-28,

and finally

  • Sphinx 1.8.2 was uploaded to PyPI on 2018-11-11.

Is this comment from the ticket description still a problem with Sphinx 1.8.2?

With Sphinx 1.8.1 we see quite a few

ApplicationError: Source directory and destination directory cannot be identical

coming out of Sphinx in conda as they have changed the argument checking of the Sphinx() constructor it seems.

comment:37 Changed 22 months ago by arojas

Unfortunately the patch no longer works with 1.8.3. I'm getting this error on the second pass:

[manifolds] building [html]: targets for 60 source files that are out of date
[manifolds] updating environment: 60 added, 0 changed, 0 removed
[manifolds]   app.builder.info(bold('loading cross citations... '), nonl=1)
[manifolds]   app.builder.info("done (%s citations)."%len(cache))
[manifolds]   app.builder.info(bold('linking _static directory.'))
[manifolds] The HTML pages are in doc/html/en/reference/manifolds.
[manifolds] Exception occurred:
[manifolds]   File "/usr/lib/python2.7/os.py", line 157, in makedirs
[manifolds]     mkdir(name, mode)
[manifolds] OSError: [Errno 17] File exists: '/build/sagemath-doc/src/sage-8.6/src/doc/html/en/reference/manifolds/_static'
[manifolds] The full traceback has been saved in /tmp/sphinx-err-EkdWJl.log, if you want to report the issue to the developers.
[manifolds] Please also report this if it was a user error, so that a better error message can be provided next time.
[manifolds] A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
Error building the documentation.
Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/build/sagemath-doc/src/sage-8.6/src/sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 1744, in main
    builder()
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 356, in _wrapper
    getattr(get_builder(document), name)(*args, **kwds)
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 547, in _wrapper
    build_many(build_ref_doc, L)
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 290, in build_many
    results.append(target(arg))
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 74, in build_ref_doc
    getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 772, in _wrapper
    getattr(DocBuilder, build_type)(self, *args, **kwds)
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/__init__.py", line 130, in f
    runsphinx()
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/sphinxbuild.py", line 319, in runsphinx
    sys.stderr.raise_errors()
  File "/build/sagemath-doc/src/sage-8.6/local-python/sage_setup/docbuild/sphinxbuild.py", line 254, in raise_errors
    raise OSError(self._error)
OSError: Exception occurred:

    Note: incremental documentation builds sometimes cause spurious
    error messages. To be certain that these are real errors, run
    "make doc-clean" first and try again.
Last edited 22 months ago by arojas (previous) (diff)

comment:38 Changed 22 months ago by arojas

The problem is that /build/sagemath-doc/src/sage-8.6/src/doc/html/en/reference/manifolds/_static is a symlink to a non-existant ../_static, which makes sphinx raise an error after this commit

comment:39 Changed 22 months ago by jhpalmieri

I don't understand what Sphinx is doing. I don't know if I will have time to work on this soon, but a starting point is to get rid of my proposed _static commit. The other commits (or variations on them – in particular environment.patch needs to be modified) are I think needed.

Here is my first question: why does the inventory builder create _static directories? Those directories should not be there in the first place. They only contain graphviz.css, which makes me think it's a bug in Sphinx's graphviz extension. A Sage-specific patch for Sphinx that might help:

  • sphinx/ext/graphviz.py

    diff --git a/sphinx/ext/graphviz.py~ b/sphinx/ext/graphviz.py
    index c9b1541..6fffcd6 100644
    a b def man_visit_graphviz(self, node): 
    410410
    411411def on_build_finished(app, exc):
    412412    # type: (Sphinx, Exception) -> None
    413     if exc is None:
     413    if exc is None and not app.config.multidoc_first_pass:
    414414        src = path.join(sphinx.package_dir, 'templates', 'graphviz', 'graphviz.css')
    415415        dst = path.join(app.outdir, '_static')
    416416        copy_asset(src, dst)

comment:40 Changed 22 months ago by jhpalmieri

That patch doesn't quite work: there is still a _static directory in local/share/doc/sage/inventory/en/reference/, although it prevents them from being created in all of the subdirectories. Something stupid like not 'inventory' in app.outdir works, although there should be better ways to check that we're doing an inventory build. That still doesn't work, of course, but it's a place to start.

comment:41 Changed 22 months ago by jhpalmieri

If I use some sort of stupid patch on graphviz.py, then Sphinx crashes when it reaches the second pass, because of the reason you point out: html/en/reference/_static does not exist, but there are symlinks pointing to it. If I then manually create this directory (which we could add to the code), the docbuild still fails:

WARNING: search index couldn't be loaded, but not all documents will be built: the index will be incomplete.

There is a method in sphinx.builders.html called load_indexer, and for reasons I don't understand, it is not finding all of the relevant files: its input is two lists, the files it thinks it should include, and the files covered by the index, and if they are not the same, it raises this warning. I added some print statements to see how these two sets differed, and I couldn't see any pattern. When I disable that warning, it gets much farther, but I'm guessing that something will be broken with either the documentation or the index.

comment:42 Changed 22 months ago by git

  • Commit changed from 65ab61522cacad7558350f3cb1f0b16d0001235e to fc3b36f017d01c198e87d146a868f1ab7eddd23d

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a5f44betrac 26451: src/misc/sphinxify fixes
fc3b36ftrac 26451:

comment:43 Changed 22 months ago by jhpalmieri

Okay, here is my current "progress". If I use the branch I just posted, along with this change to Sphinx (which is a terrible idea):

  • sphinx/builders/html.py

    diff --git a/sphinx/builders/html.py b/sphinx/builders/html.py
    index 5060d1f..6747b6c 100644
    a b class StandaloneHTMLBuilder(Builder): 
    10101010            with f:
    10111011                self.indexer.load(f, self.indexer_format)
    10121012        except (IOError, OSError, ValueError):
    1013             if keep:
     1013            if keep and False:
    10141014                logger.warning(__('search index couldn\'t be loaded, but not all '
    10151015                                  'documents will be built: the index will be '
    10161016                                  'incomplete.'))

then the documentation builds. Remaining questions: is the documentation complete? Is the index complete? What is the right way to fix the problem that this stupid patch fixes? Are the other changes okay?

comment:44 Changed 22 months ago by jdemeyer

  • Dependencies changed from #26558 to #26949
  • Description modified (diff)
  • Summary changed from Upgrade to Sphinx 1.8.2 to Upgrade to Sphinx 1.8.3

This really should be rebased to #26949 since that makes non-trivial changes to the docbuilder.

comment:45 Changed 22 months ago by jhpalmieri

Note that the line from #26949

from sphinx.ext.autodoc.inspector import format_annotation, formatargspec

and later use of formatargspec leads to deprecation warnings with new versions of Sphinx.

comment:46 Changed 22 months ago by jhpalmieri

On the other hand, documentation seems to build with the branch here + #26949. Now to figure out which changes are actually needed once rebased to #26949.

comment:47 Changed 21 months ago by git

  • Commit changed from fc3b36f017d01c198e87d146a868f1ab7eddd23d to 54179cdace30db612c53e2626beab20c2e09f903

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4a3ef7fMake sage autodoc work for both py2 and py3
54179cdtrac 26451: Sphinx 1.8.3

comment:48 Changed 21 months ago by jhpalmieri

  • Authors changed from Julian Rüth, Jeroen Demeyer to Julian Rüth, Jeroen Demeyer, John Palmieri
  • Status changed from needs_info to needs_review

Okay, let's try this.

comment:49 Changed 21 months ago by git

  • Commit changed from 54179cdace30db612c53e2626beab20c2e09f903 to 08a92c96413f26e1a63fe346ab98fd5265cd0ef5

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

08a92c9trac 26451: patch Sphinx's graphviz.py

comment:50 Changed 21 months ago by jdemeyer

I'd rather wait until the dependency is actually merged to review this.

comment:51 Changed 21 months ago by gh-timokau

What's the upstream story of that patch? Was it proposed somewhere? What is its purpose?

comment:52 Changed 21 months ago by jhpalmieri

For its purpose, see comments 25-30, 39, and maybe some others. (Search this page for "graphviz".) Basically, Sphinx needlessly copies the file graphviz.css to inventory/en/refererence/MODULE/_static/ directories, and then that confuses the intersphinx build. I haven't reported it upstream because I don't know how Sage-specific it is. We have our own inventory builder, our own "multidocs" extension, etc., and it is not clear how much those are responsible for the problem.

I would also not be surprised if there were better ways to handle the problem with graphviz.css. I believe that my suggestion works, so now is the time to confirm that and then to improve on it. (Or wait until #26949 is merged and then do that.)

comment:53 Changed 21 months ago by jdemeyer

  • Branch changed from u/jhpalmieri/26451 to u/jdemeyer/26451

comment:54 Changed 21 months ago by jdemeyer

  • Commit changed from 08a92c96413f26e1a63fe346ab98fd5265cd0ef5 to c7248a2c9737e1af66fadee939c281f94a16f3fe
  • Dependencies #26949 deleted

Rebased to 8.7.beta1


New commits:

fe643beUpgrade to Sphinx 1.8.1
e7aa042Call Sphinx() 1.8 constructor
fd941c1trac 26451: Sphinx 1.8.3
c7248a2trac 26451: patch Sphinx's graphviz.py

comment:55 Changed 21 months ago by jdemeyer

I also restored some of the older commits, to make clear who did what.

comment:56 Changed 21 months ago by jdemeyer

Based on the diff, I'm willing to give this ticket the benefit of the doubt. I cannot say that I understand everything but there is nothing obviously bad.

I haven't actually built the documentation yet, I'll do that next.

comment:57 follow-up: Changed 21 months ago by arojas

As a packager I share Timo's concerns about the sphinx patch. Are there plans to upstream it?

comment:58 in reply to: ↑ 57 ; follow-up: Changed 21 months ago by jdemeyer

Replying to arojas:

As a packager I share Timo's concerns about the sphinx patch.

Is there is any particular reason that you're concerned about that specific patch and not the other patches that Sage has for Sphinx?

comment:59 in reply to: ↑ 58 ; follow-up: Changed 21 months ago by arojas

Replying to jdemeyer:

Replying to arojas:

As a packager I share Timo's concerns about the sphinx patch.

Is there is any particular reason that you're concerned about that specific patch and not the other patches that Sage has for Sphinx?

Yes, those other patches either address minor issues or are not relevant to Arch, so not having them is not a problem. This one is necessary for docs to build at all.

comment:60 follow-up: Changed 21 months ago by jdemeyer

There is a minor regression in the produced documentation: a <BLANKLIKE> in a doctest is now rendered as <BLANKLINE> instead of an actual blank line.

comment:61 Changed 21 months ago by jdemeyer

That's the only issue in the generated documentation I could find by diffing the docs before and after this ticket. It does have problems with parsing `A = ` ``self`` (see src/sage/categories/filtered_algebras_with_basis.py) but the current version also has problems, just different problems.

comment:62 in reply to: ↑ 60 Changed 21 months ago by jhpalmieri

Replying to jdemeyer:

There is a minor regression in the produced documentation: a <BLANKLIKE> in a doctest is now rendered as <BLANKLINE> instead of an actual blank line.

Did you check to see if other nonmath_substitutes from misc.sagedoc are handled properly? (Or I should say, at least as properly as they currently are?)

Last edited 21 months ago by jhpalmieri (previous) (diff)

comment:63 Changed 21 months ago by jhpalmieri

Maybe this isn't handled by sage.misc.sagedoc, but should instead be handled by sphinx.ext.doctest: lines like

        if self.name == 'doctest':
            if '<BLANKLINE>' in code:
                # convert <BLANKLINE>s to ordinary blank lines for presentation
                test = code
                code = blankline_re.sub('', code)

There weren't any changes in this between the old and new versions of Sphinx.

comment:64 Changed 21 months ago by jhpalmieri

  • Branch changed from u/jdemeyer/26451 to u/jhpalmieri/26451

comment:65 in reply to: ↑ 59 Changed 21 months ago by jhpalmieri

  • Commit changed from c7248a2c9737e1af66fadee939c281f94a16f3fe to 5f7b6ecf8b4596fb497642b71cd87b140428fbab

Replying to arojas:

Replying to jdemeyer:

Replying to arojas:

As a packager I share Timo's concerns about the sphinx patch.

Is there is any particular reason that you're concerned about that specific patch and not the other patches that Sage has for Sphinx?

Yes, those other patches either address minor issues or are not relevant to Arch, so not having them is not a problem. This one is necessary for docs to build at all.

Here is maybe a better approach with graphviz.


New commits:

0347a79trac 26461: inventory builder: remove _static directory upon cleanup
5f7b6ectrac 26451: change 'app.builder.info' to 'logger.info'

comment:66 follow-up: Changed 21 months ago by jhpalmieri

The point here is that the inventory builder is Sage-specific, and that's where the problem lies. So patching graphviz.py to deal with the inventory builder is okay (in my opinion) and would not make sense to report upstream. It looks like we can deal with it differently, though: rather than preventing graphviz.py from populating _static directories, we can remove them in the cleanup stage of the inventory build.

comment:67 Changed 21 months ago by jdemeyer

Great! I'll look into the <BLANKLINE> thing now.

comment:68 in reply to: ↑ 66 Changed 21 months ago by arojas

Replying to jhpalmieri:

The point here is that the inventory builder is Sage-specific, and that's where the problem lies. So patching graphviz.py to deal with the inventory builder is okay (in my opinion) and would not make sense to report upstream. It looks like we can deal with it differently, though: rather than preventing graphviz.py from populating _static directories, we can remove them in the cleanup stage of the inventory build.

Thanks. With this branch, the docs build fine on Arch with unpatched sphinx 1.8.3, modulo #26608

comment:69 Changed 21 months ago by git

  • Commit changed from 5f7b6ecf8b4596fb497642b71cd87b140428fbab to a3ceda240bb3e8bbb840d8635e5a62dc240d858a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a3568fdtrac 26461: inventory builder: remove _static directory upon cleanup
a3ceda2trac 26451: change 'app.builder.info' to 'logger.info'

comment:70 Changed 21 months ago by jhpalmieri

The most recent commit is an attempt on my part to squash the commits that add and then remove the graphviz patch. My first time squashing, so I hope it worked.

comment:71 Changed 21 months ago by thansen

Thanks for avoiding the graphviz.py patch. Now we can also use the patch in Debian.

In Debian we have a few new failing doctests with sage 8.6, sphinx 1.8.3 and this patch. There is this:

sage -t --long src/doc/common/conf.py
**********************************************************************
File "src/doc/common/conf.py", line 619, in doc.common.conf.call_intersphinx
Failed example:
    for line in open(thematic_index).readlines():  # optional - dochtml
        if "padics" in line:
            sys.stdout.write(line)
Expected:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(in Sage Reference Manual: p-Adics v...)"><span>Introduction to the -adics</span></a></li>
Got:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(in Sage Reference Manual: p-Adics v8.6)"><span>Introduction to the p-adics</span></a></li>

and 6 occurances of ApplicationError: Source directory and destination directory cannot be identical in sagenb/misc/sphinxify.py (which need to be fixed in sagenb I guess).

comment:72 follow-up: Changed 21 months ago by slelievre

  • Cc dimpase added
  • Milestone changed from sage-8.5 to sage-8.7

Cc Dima Pasechnik about the sagenb question.

comment:73 in reply to: ↑ 72 Changed 21 months ago by dimpase

Replying to slelievre:

Cc Dima Pasechnik about the sagenb question.

Thanks. This is now https://github.com/sagemath/sagenb/issues/465

comment:74 Changed 21 months ago by git

  • Commit changed from a3ceda240bb3e8bbb840d8635e5a62dc240d858a to 62b989d5ee1d9646db85ea56053cd22e9ffde5ab

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

62b989dtrac 26451: fix doctest in src/doc/common/conf.py

comment:75 follow-up: Changed 21 months ago by jhpalmieri

The SageNB problem should be fixed from their end, and here is a fix for the Sage doctest failure.

comment:76 in reply to: ↑ 75 Changed 21 months ago by dimpase

Replying to jhpalmieri:

The SageNB problem should be fixed from their end, and here is a fix for the Sage doctest failure.

It is fixed now (the fix will be the next sagenb release).

comment:77 Changed 21 months ago by dimpase

  • Status changed from needs_review to needs_work

One annoying warning with this sphinx now:

src/sage_setup/docbuild/ext/sage_autodoc.py:1170: RemovedInSphinx20Warning: formatargspec() is now deprecated.  Please use sphinx.util.inspect.Signature 

(and the same warning in sage_autodoc.py:1072)

Last edited 21 months ago by dimpase (previous) (diff)

comment:78 Changed 21 months ago by jhpalmieri

Noted in comment:45. It's annoying but can we deal with it on another ticket?

Last edited 21 months ago by jhpalmieri (previous) (diff)

comment:79 Changed 21 months ago by jhpalmieri

In particular, I don't know how to solve it. Signature does not seem to be just a drop-in replacement. We could just silence the warnings, but that's probably a bad idea.

comment:80 Changed 21 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_work to positive_review

I agree, it needs a bit of coding (examples may be found by googling this warning...) A new ticket then. By the way, the sagenb update is on #27200.

comment:81 Changed 21 months ago by jhpalmieri

Jeroen, do you agree with the positive review?

comment:82 Changed 21 months ago by jdemeyer

  • Status changed from positive_review to needs_review

Is the <BLANKLINE> issue fixed?

comment:83 Changed 21 months ago by dimpase

to me, blankline is blankline, how to check this...

comment:84 Changed 21 months ago by dimpase

  • Dependencies set to #27200

some of the doctests here are fixed by #27200, adding as a dependency

comment:85 Changed 21 months ago by jhpalmieri

  • Status changed from needs_review to needs_work

Two problems:

  • The BLANKLINE problem is still there. For example, in calculus/riemann.pyx, a doctest for complex_to_spiderweb includes <BLANKLINE>. With the old Sphinx, this was printed as an actual blank line, while with the new one, it prints "<BLANKLINE>".
  • The real blocker: math is not rendered in the reference manual (although it is in the tutorial). Indeed, in the generated html files, these lines were included in the old Sphinx, but not in the new one. I don't know why, and I don't know if I have time figure it out right now:
        <script type="text/javascript" src="../../../_static/MathJax.js?config=TeX-AMS_HTML-full,../mathjax_sage.js"></script>
        <script type="text/javascript" src="../../../_static/MathJax.js?config=TeX-AMS_HTML-full,../mathjax_sage.js"></script>
    

Manually adding those lines fixes the problem, but they should be there automatically.

comment:86 Changed 21 months ago by dimpase

I tried to replace src/doc/common/themes/sageref with a symlink to src/doc/common/themes/sage, but this did not change the result (no TeX in reference manual).

Could it be some special parallel hack that breaks it? I really have no idea.

comment:87 Changed 21 months ago by jhpalmieri

I think the issue is that in the new Sphinx, sphinx.ext.mathjax has the line

    app.connect('env-check-consistency', install_mathjax)

As far as I can tell, this means that those lines should be produced when self.env.check_consistency() is run, but it doesn't get run during the reference/html build. I'm guessing this is because of our multidocs extension, or maybe the sage_autodoc extension.

comment:88 Changed 21 months ago by thansen

  • Cc thansen added

comment:90 Changed 21 months ago by jhpalmieri

Unfortunately this doesn't help with the mathjax issue. I'm stuck, so I hope someone else has some ideas.

comment:91 Changed 20 months ago by git

  • Commit changed from 62b989d5ee1d9646db85ea56053cd22e9ffde5ab to d566fc954df56bf61efab7eaf25d7ee87fee5f11

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

bc5d0abUpgrade to Sphinx 1.8.1
8a55761Call Sphinx() 1.8 constructor
d3b6076trac 26451: Sphinx 1.8.3
09a1415trac 26461: inventory builder: remove _static directory upon cleanup
0ed8d9etrac 26451: change 'app.builder.info' to 'logger.info'
caef351trac 26451: upgrade to Sphinx 1.8.4.
0b93bc2trac 26451: patch Sphinx so it recognizes "sage:"
d566fc9trac 26451: emit 'env-check-consistency' if not first pass building ref manual,

comment:92 follow-up: Changed 20 months ago by jhpalmieri

Okay, I've fixed the mathjax problem and the <BLANKLINE> problem. The second of these involves patching Sphinx, so if you don't want patches, you will get <BLANKLINE> in some doctest output, rather than an actual blank line. Sphinx still produces lots of formatargspec() is now deprecated warnings, but I suggest we deal with those in a separate ticket.

Oh, by the way, Sphinx 2.0.0.beta1 has been released. It drops Python 2.7 support, so it's not for us yet.

comment:93 Changed 20 months ago by jhpalmieri

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

comment:94 Changed 20 months ago by slelievre

  • Description modified (diff)
  • Keywords upgrade added
  • Summary changed from Upgrade to Sphinx 1.8.3 to Upgrade to Sphinx 1.8.4

comment:95 Changed 20 months ago by jhpalmieri

If all else fails, we can just silence those deprecation warnings the same way we silence lots of other "chatter" in the Sphinx build. See sage_setup/docbuild/sphinxbuild.py.

comment:96 follow-up: Changed 20 months ago by gh-timokau

Why wasn't the sage: prompt patch necessary before?

On nixos with this patch applied I get some kind of unicode issue (not sure if its 100% reproducible; I think I've seen it working some time):

sage -t docsrc/common/conf.py
**********************************************************************
File "docsrc/common/conf.py", line 614, in common.conf.call_intersphinx
Failed example:
    for line in open(thematic_index).readlines():  # optional - dochtml
        if "padics" in line:
            sys.stdout.write(line)
Expected:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(i
n Sage Reference Manual: p-Adics v...)"><span>Introduction to the p-adics</span></a></li>
Got:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(в
 Sage Reference Manual: p-Adics v8.6)"><span>Introduction to the p-adics</span></a></li>
**********************************************************************
1 item had failures:
   1 of   4 in common.conf.call_intersphinx
    [3 tests, 1 failure, 0.01 s]

I don't know if this has to do with some different dependency on nix or if it could also happen on sage-the-distro.

comment:97 in reply to: ↑ 96 Changed 20 months ago by jhpalmieri

Replying to gh-timokau:

Why wasn't the sage: prompt patch necessary before?

Because the file being patched didn't exist before.

On nixos with this patch applied I get some kind of unicode issue (not sure if its 100% reproducible; I think I've seen it working some time):

sage -t docsrc/common/conf.py
**********************************************************************
File "docsrc/common/conf.py", line 614, in common.conf.call_intersphinx
Failed example:
    for line in open(thematic_index).readlines():  # optional - dochtml
        if "padics" in line:
            sys.stdout.write(line)
Expected:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(i
n Sage Reference Manual: p-Adics v...)"><span>Introduction to the p-adics</span></a></li>
Got:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(в
 Sage Reference Manual: p-Adics v8.6)"><span>Introduction to the p-adics</span></a></li>
**********************************************************************
1 item had failures:
   1 of   4 in common.conf.call_intersphinx
    [3 tests, 1 failure, 0.01 s]

I don't know if this has to do with some different dependency on nix or if it could also happen on sage-the-distro.

I don't see this with sage-the-distro. The file doc/common/conf.py had its doctests changed recently in another ticket (to fix Python 3 issues, I think). This branch doesn't touch that file, and I don't see why it would be related.

comment:98 in reply to: ↑ 92 Changed 20 months ago by thansen

Replying to jhpalmieri:

Okay, I've fixed the mathjax problem and the <BLANKLINE> problem. The second of these involves patching Sphinx, so if you don't want patches, you will get <BLANKLINE> in some doctest output, rather than an actual blank line.

Instead of adding this very sage specific patch to sphinx, why not do this in a sphinx extension?

Like this:

--- a/src/doc/common/conf.py
+++ b/src/doc/common/conf.py
@@ -13,7 +13,7 @@
 
 # Add any Sphinx extension module names here, as strings. They can be extensions
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
-extensions = ['inventory_builder', 'multidocs',
+extensions = ['inventory_builder', 'multidocs', 'trim_doctest_flags',
               'sage_autodoc',  'sphinx.ext.graphviz',
               'sphinx.ext.inheritance_diagram', 'sphinx.ext.todo',
               'sphinx.ext.extlinks', 'matplotlib.sphinxext.plot_directive']
--- /dev/null
+++ b/src/sage_setup/docbuild/ext/trim_doctest_flags.py
@@ -0,0 +1,67 @@
+"""
+    trim doctest flags
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    transforms for code-blocks.
+
+    modified from sphinx.transforms.post_transforms.code
+
+    :copyright: Copyright 2007-2019 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+
+import sys
+from typing import NamedTuple
+
+from docutils import nodes
+from pygments.lexers import PythonConsoleLexer, guess_lexer
+
+from sphinx import addnodes
+from sphinx.ext import doctest
+from sphinx.transforms import SphinxTransform
+
+class TrimDoctestFlagsTransformSage(SphinxTransform):
+    """
+    Trim doctest flags like ``# doctest: +FLAG`` from sage code-blocks.
+
+    The only difference from Sphinx' TrimDoctestFlagsTransform is that
+    here lines starting with 'sage:' rather than '>>>' are transformed.
+    """
+    default_priority = 401
+
+    def apply(self, **kwargs):
+        # type: (Any) -> None
+        if not self.config.trim_doctest_flags:
+            return
+
+        for node in self.document.traverse(nodes.literal_block):
+            if self.is_pyconsole(node):
+                source = node.rawsource
+                source = doctest.blankline_re.sub('', source)
+                source = doctest.doctestopt_re.sub('', source)
+                node.rawsource = source
+                node[:] = [nodes.Text(source)]
+
+    @staticmethod
+    def is_pyconsole(node):
+        # type: (nodes.literal_block) -> bool
+        if node.rawsource != node.astext():
+            return False  # skip parsed-literal node
+
+        language = node.get('language')
+        if language in ('pycon', 'pycon3'):
+            return True
+        elif language in ('py', 'py3', 'python', 'python3', 'default'):
+            return node.rawsource.startswith('sage:')
+        elif language == 'guess':
+            try:
+                lexer = guess_lexer(node.rawsource)
+                return isinstance(lexer, PythonConsoleLexer)
+            except Exception:
+                pass
+
+        return False
+
+
+def setup(app):
+    app.add_post_transform(TrimDoctestFlagsTransformSage)

https://salsa.debian.org/science-team/sagemath/blob/master/debian/patches/u1-sphinx-1.8-doctest-transform.patch

comment:99 Changed 20 months ago by jhpalmieri

There is a balance to be struck: we're more or less forking Sphinx with extensions like this, so they require maintenance: as Sphinx changes, so should the extension. This recently required a fair amount of work for Sage's autodoc extension (#26949).

So is it better to patch Sphinx or create an extension?

comment:100 follow-up: Changed 20 months ago by jhpalmieri

Any more comments on this? If people who deal with distributions feel that using a Sphinx extension would be better than a patch, then please go ahead and implement that. If people feel that patching is better, then what else needs to be done?

comment:101 in reply to: ↑ 100 ; follow-ups: Changed 20 months ago by thansen

Replying to jhpalmieri:

Any more comments on this? If people who deal with distributions feel that using a Sphinx extension would be better than a patch, then please go ahead and implement that. If people feel that patching is better, then what else needs to be done?

I already implemented it and posted the patch here. Is there something wrong with it?

comment:102 in reply to: ↑ 101 Changed 20 months ago by dimpase

Replying to thansen:

Replying to jhpalmieri:

Any more comments on this? If people who deal with distributions feel that using a Sphinx extension would be better than a patch, then please go ahead and implement that. If people feel that patching is better, then what else needs to be done?

I already implemented it and posted the patch here. Is there something wrong with it?

Care to propose a full branch? (So that it's 100% clear what the solution is)

comment:103 in reply to: ↑ 101 Changed 20 months ago by jhpalmieri

Replying to thansen:

Replying to jhpalmieri:

Any more comments on this? If people who deal with distributions feel that using a Sphinx extension would be better than a patch, then please go ahead and implement that. If people feel that patching is better, then what else needs to be done?

I already implemented it and posted the patch here. Is there something wrong with it?

Not necessarily, but I did respond and ask a question about the philosophy of patches vs. extensions. I was hoping for a discussion.

comment:104 Changed 20 months ago by thansen

From a packagers standpoint the extension is clearly better. Since the sphinx patch does not make much sense for us we would probably have to keep patching in the extension anyway. But I thought that was obvious and you were asking for opinions of sage developers.

comment:105 follow-up: Changed 20 months ago by dimpase

From developers point of view, we appreciate your efforts a lot --- although we'd much prefer a branch with your solution to a link to Debian patches, from which one sometimes might need to do reverse engineering to get what's needed and works in Sage sans Debian...

comment:106 in reply to: ↑ 105 ; follow-up: Changed 20 months ago by thansen

Replying to dimpase:

From developers point of view, we appreciate your efforts a lot --- although we'd much prefer a branch with your solution to a link to Debian patches, from which one sometimes might need to do reverse engineering to get what's needed and works in Sage sans Debian...

Ok, here is a branch: https://git.sagemath.org/sage.git/log/?h=u/thansen/26451

comment:107 in reply to: ↑ 106 ; follow-up: Changed 20 months ago by fbissey

Replying to thansen:

Replying to dimpase:

From developers point of view, we appreciate your efforts a lot --- although we'd much prefer a branch with your solution to a link to Debian patches, from which one sometimes might need to do reverse engineering to get what's needed and works in Sage sans Debian...

Ok, here is a branch: https://git.sagemath.org/sage.git/log/?h=u/thansen/26451

Clarification. I am assuming this needs to be applied on top of the current branch already in this ticket. Right?

comment:108 in reply to: ↑ 107 Changed 20 months ago by thansen

Replying to fbissey:

Replying to thansen:

Replying to dimpase:

From developers point of view, we appreciate your efforts a lot --- although we'd much prefer a branch with your solution to a link to Debian patches, from which one sometimes might need to do reverse engineering to get what's needed and works in Sage sans Debian...

Ok, here is a branch: https://git.sagemath.org/sage.git/log/?h=u/thansen/26451

Clarification. I am assuming this needs to be applied on top of the current branch already in this ticket. Right?

No, this is just the branch of this ticket with John's fix for the <BLANKLINE> issue removed and my patch applied.

comment:109 follow-up: Changed 20 months ago by dimpase

So, building reference manual, mathjax works on u/thansen/26451

comment:110 in reply to: ↑ 109 ; follow-up: Changed 20 months ago by thansen

Replying to dimpase:

So, building reference manual, mathjax works on u/thansen/26451

The thing I changed has nothing to do with mathjax, it's a different fix for the <BLANKLINE> issue.

comment:111 in reply to: ↑ 110 ; follow-ups: Changed 20 months ago by dimpase

Replying to thansen:

Replying to dimpase:

So, building reference manual, mathjax works on u/thansen/26451

The thing I changed has nothing to do with mathjax, it's a different fix for the <BLANKLINE> issue.

Sorry, I got lost here. Is u/thansen/26451 a complete branch? If so, please put it into the Branch field of the ticket, and add your name to the list of authors.

comment:112 in reply to: ↑ 111 Changed 20 months ago by fbissey

Replying to dimpase:

Replying to thansen:

Replying to dimpase:

So, building reference manual, mathjax works on u/thansen/26451

The thing I changed has nothing to do with mathjax, it's a different fix for the <BLANKLINE> issue.

Sorry, I got lost here. Is u/thansen/26451 a complete branch? If so, please put it into the Branch field of the ticket, and add your name to the list of authors.

That's why I asked the question earlier. I was going to do just that on thansen's behalf, but when I clicked on the link I could only see one commit. Now that may just have been the way I navigated there, that's why I asked.

comment:113 in reply to: ↑ 111 Changed 20 months ago by thansen

Replying to dimpase:

Replying to thansen:

Replying to dimpase:

So, building reference manual, mathjax works on u/thansen/26451

The thing I changed has nothing to do with mathjax, it's a different fix for the <BLANKLINE> issue.

Sorry, I got lost here. Is u/thansen/26451 a complete branch? If so, please put it into the Branch field of the ticket, and add your name to the list of authors.

Yes, it's a complete branch. I didn't put it in the branch field of the ticket because this is John's ticket and he never said we should use my fix instead of his.

comment:114 Changed 20 months ago by dimpase

  • Authors changed from Julian Rüth, Jeroen Demeyer, John Palmieri to Julian Rüth, Jeroen Demeyer, John Palmieri, Tobias Hansen
  • Branch changed from u/jhpalmieri/26451 to u/thansen/26451
  • Commit changed from d566fc954df56bf61efab7eaf25d7ee87fee5f11 to c17bbf83d2cdf7d748fd9fa4d3b10348bce3ed27
  • Dependencies #27200 deleted

IMHO John won't mind---in fact he appears to have said OK to this on comment:101. (John, please shout if I am wrong).


New commits:

dca2e53trac 26451: emit 'env-check-consistency' if not first pass building ref manual,
c17bbf8trac 26451: Add sphinx extension to modify lines in codeblocks starting with 'sage:'

comment:115 Changed 19 months ago by jhpalmieri

  • Branch changed from u/thansen/26451 to u/jhpalmieri/26451

comment:116 Changed 19 months ago by jhpalmieri

  • Commit changed from c17bbf83d2cdf7d748fd9fa4d3b10348bce3ed27 to 0dfcab9dfbc4d403c77cf058df1ef0502d0b2eff
  • Dependencies set to #27528

Here is a different approach for the <BLANKLINE> problem, as suggested on Sphinx's github page. I've added a dependency, #27528, which also removes highlighting.patch. It is necessary here because it suppresses some warnings which are produced by SageMathTransform.

If this is acceptable, we should probably squash some commits: we don't need to create trim_doctest_flags.py and then delete it.


New commits:

ea84309Upgrade to Sphinx 1.8.1
f93af93Call Sphinx() 1.8 constructor
d559c88trac 26451: Sphinx 1.8.3
e912974trac 26461: inventory builder: remove _static directory upon cleanup
89332f4trac 26451: change 'app.builder.info' to 'logger.info'
35e7862trac 26451: upgrade to Sphinx 1.8.4.
3b96fa8trac 26451: emit 'env-check-consistency' if not first pass building ref manual,
550e469trac 26451: Add sphinx extension to modify lines in codeblocks starting with 'sage:'
0dfcab9trac 26451: use a Sphinx transform module to deal with <BLANKLINE>

comment:117 follow-up: Changed 19 months ago by arojas

With this branch on top of 8.7 and sphinx 1.8.5 I'm getting:

Process Process-79:
Traceback (most recent call last):
  File "/usr/lib/python2.7/multiprocessing/process.py", line 267, in _bootstrap
    self.run()
  File "/usr/lib/python2.7/multiprocessing/process.py", line 114, in run
    self._target(*self._args, **self._kwargs)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 79, in build_ref_doc
    getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 763, in _wrapper
    getattr(DocBuilder, build_type)(self, *args, **kwds)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 135, in f
    runsphinx()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/sphinxbuild.py", line 314, in runsphinx
    sys.stderr.raise_errors()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/sphinxbuild.py", line 249, in raise_errors
    raise OSError(self._error)
OSError: /build/sagemath-doc/src/sage-8.7/src/doc/en/reference/combinat/sage/combinat/shifted_primed_tableau.rst:267: WARNING: Could not lex literal_block as "pycon". Highlighting skipped.
Error building the documentation.
Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/build/sagemath-doc/src/sage-8.7/src/sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 1732, in main
    builder()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 358, in _wrapper
    getattr(get_builder(document), name)(*args, **kwds)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 549, in _wrapper
    build_many(build_ref_doc, L)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/utils.py", line 214, in _build_many
    raise worker_exc
WorkerDiedException: worker for ('reference/combinat', 'en', 'html', {}) died with non-zero exit code 1

    Note: incremental documentation builds sometimes cause spurious
    error messages. To be certain that these are real errors, run
    "make doc-clean" first and try again.

comment:118 Changed 19 months ago by slelievre

Sphinx 1.8.5 was released on PyPI on 2019-03-10, and Sphinx 2.0.0b2 on 2019-03-20. See

comment:119 Changed 19 months ago by jhpalmieri

I believe that Sphinx 2.0.0b2 is dropping support for Python 2.7, so we can't use that. I'll look at 1.8.5.

comment:120 in reply to: ↑ 117 ; follow-up: Changed 19 months ago by jhpalmieri

Replying to arojas:

With this branch on top of 8.7 and sphinx 1.8.5 I'm getting:

Process Process-79:
Traceback (most recent call last):
  File "/usr/lib/python2.7/multiprocessing/process.py", line 267, in _bootstrap
    self.run()
  File "/usr/lib/python2.7/multiprocessing/process.py", line 114, in run
    self._target(*self._args, **self._kwargs)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 79, in build_ref_doc
    getattr(ReferenceSubBuilder(doc, lang), format)(*args, **kwds)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 763, in _wrapper
    getattr(DocBuilder, build_type)(self, *args, **kwds)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 135, in f
    runsphinx()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/sphinxbuild.py", line 314, in runsphinx
    sys.stderr.raise_errors()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/sphinxbuild.py", line 249, in raise_errors
    raise OSError(self._error)
OSError: /build/sagemath-doc/src/sage-8.7/src/doc/en/reference/combinat/sage/combinat/shifted_primed_tableau.rst:267: WARNING: Could not lex literal_block as "pycon". Highlighting skipped.
Error building the documentation.
Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/build/sagemath-doc/src/sage-8.7/src/sage_setup/docbuild/__main__.py", line 2, in <module>
    main()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 1732, in main
    builder()
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 358, in _wrapper
    getattr(get_builder(document), name)(*args, **kwds)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/__init__.py", line 549, in _wrapper
    build_many(build_ref_doc, L)
  File "/build/sagemath-doc/src/sage-8.7/local-python/sage_setup/docbuild/utils.py", line 214, in _build_many
    raise worker_exc
WorkerDiedException: worker for ('reference/combinat', 'en', 'html', {}) died with non-zero exit code 1

    Note: incremental documentation builds sometimes cause spurious
    error messages. To be certain that these are real errors, run
    "make doc-clean" first and try again.

Did you also use the branch from #27528? That would get rid of the warning which is causing the problems. This is somewhat moot, since I am reworking #27528. I should be able to post something later today.

comment:121 in reply to: ↑ 120 ; follow-up: Changed 19 months ago by arojas

Replying to jhpalmieri:

Did you also use the branch from #27528? That would get rid of the warning which is causing the problems. This is somewhat moot, since I am reworking #27528. I should be able to post something later today.

Ah, no. I was under the (wrong) impression that branches in trac tickets already pulled the changes from dependant tickets.

comment:122 in reply to: ↑ 121 Changed 19 months ago by jhpalmieri

Replying to arojas:

Replying to jhpalmieri:

Did you also use the branch from #27528? That would get rid of the warning which is causing the problems. This is somewhat moot, since I am reworking #27528. I should be able to post something later today.

Ah, no. I was under the (wrong) impression that branches in trac tickets already pulled the changes from dependant tickets.

Sometimes they do and sometimes they don't. I don't know what I am supposed to do when preparing branches: base this one on the one from #27528 or on develop?

comment:123 Changed 19 months ago by jhpalmieri

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

comment:124 Changed 19 months ago by git

  • Commit changed from 0dfcab9dfbc4d403c77cf058df1ef0502d0b2eff to 0a92d485970b31b8fb4b70a589d063534b9ab936

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

48b7acdtrac 27528: remove some patches for Sphinx and pygments.
b23b8cctrac 27528: use a custom Sphinx transform to handle <BLANKLINE>
687b338trac 27528: clean up some code-blocks in the reST documentation.
e1794bbtrac 26451: upgrade to Sphinx 1.8.5.
c61a041trac 26451: Call Sphinx() 1.8 constructor
5e32410trac 26451: change 'app.builder.info' to 'logger.info'.
0a92d48trac 26451: inventory builder: remove _static directory upon cleanup,

comment:125 Changed 19 months ago by jhpalmieri

  • Status changed from needs_work to needs_review

Here is a new branch which is based on #27528. I put the <BLANKLINE> stuff at #27528, not here, since I needed it there.

comment:126 Changed 19 months ago by jhpalmieri

  • Summary changed from Upgrade to Sphinx 1.8.4 to Upgrade to Sphinx 1.8.5

comment:127 Changed 19 months ago by embray

  • Milestone changed from sage-8.7 to sage-8.8

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

comment:128 Changed 19 months ago by jhpalmieri

  • Description modified (diff)

comment:129 Changed 19 months ago by git

  • Commit changed from 0a92d485970b31b8fb4b70a589d063534b9ab936 to 737afd8f314bd1e16feaec562bb4b5efa2effa8b

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

737afd8trac 26451: fix one doctest

comment:130 Changed 19 months ago by jhpalmieri

PDF documentation builds fine for me.

comment:131 Changed 19 months ago by dimpase

  • Status changed from needs_review to positive_review

looks good to me.

comment:132 Changed 19 months ago by jhpalmieri

Great! Now we need to silence all of the stupid deprecation warnings.

comment:133 Changed 19 months ago by jhpalmieri

See #27578 for a ticket to stop using the deprecated formatargspec.

comment:134 Changed 19 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:135 Changed 19 months ago by git

  • Commit changed from 737afd8f314bd1e16feaec562bb4b5efa2effa8b to f1a08e3522b66afe96e8fdd389cf1114b1db1365

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a55438etrac 26451: upgrade to Sphinx 1.8.5.
827ff80trac 26451: Call Sphinx() 1.8 constructor
22736d7trac 26451: change 'app.builder.info' to 'logger.info'.
4cab150trac 26451: inventory builder: remove _static directory upon cleanup,
f1a08e3trac 26451: fix one doctest

comment:136 Changed 19 months ago by jhpalmieri

  • Status changed from needs_work to positive_review

Rebased.

comment:137 Changed 19 months ago by vbraun

  • Branch changed from u/jhpalmieri/26451 to f1a08e3522b66afe96e8fdd389cf1114b1db1365
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.