Opened 5 years ago

Closed 5 years ago

#24935 closed enhancement (fixed)

Upgrade to Sphinx 1.7.5

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.3
Component: packages: standard Keywords:
Cc: embray, fbissey, dimpase Merged in:
Authors: Jeroen Demeyer Reviewers: Ralf Stephan, François Bissey
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: 4e066ba (Commits, GitHub, GitLab) Commit: 4e066baffdce49ccc9bcc402b1ff534022b5f355
Dependencies: #25570 Stopgaps:

Status badges

Change History (62)

comment:1 Changed 5 years ago by fbissey

Cc: fbissey added

comment:2 Changed 5 years ago by jdemeyer

Dependencies: #24972

comment:3 Changed 5 years ago by jdemeyer

Description: modified (diff)

comment:4 Changed 5 years ago by jdemeyer

Branch: u/jdemeyer/upgrade_to_sphinx_1_7_1

comment:5 Changed 5 years ago by jdemeyer

Commit: 41f114ec76d1e97701f77e0b197615faab308a10
Description: modified (diff)

New commits:

f5dd6dbadded sphinxcontrib_websupport
8ecd93cMinor fixes to sphinxcontrib_websupport
41f114eUpgrade to Sphinx 1.7.1

comment:6 Changed 5 years ago by git

Commit: 41f114ec76d1e97701f77e0b197615faab308a1083682b257e42815d8608db5440f63983ad723088

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

83682b2Upgrade to Sphinx 1.7.1

comment:7 Changed 5 years ago by git

Commit: 83682b257e42815d8608db5440f63983ad7230884fc721aa2203e9c4ff34df1acedcd70f443b96b9

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

4fc721aUpgrade to Sphinx 1.7.1

comment:8 Changed 5 years ago by jdemeyer

Status: newneeds_review

comment:9 Changed 5 years ago by rws

Can you please give a link to the sphinxcontrib_websupport package, or is it supposed to be installed by other means? An upgrade attempt failed here because the mirrors didn't have it.

comment:10 in reply to:  9 Changed 5 years ago by jdemeyer

Replying to rws:

Can you please give a link to the sphinxcontrib_websupport package, or is it supposed to be installed by other means?

It's linked on the dependency #24972

comment:11 Changed 5 years ago by rws

Status: needs_reviewneeds_info
**********************************************************************
File "src/sage/misc/sphinxify.py", line 51, in sage.misc.sphinxify.sphinxify
Failed example:
    sphinxify('**Testing**\n`monospace`')
Expected:
    '...<div class="docstring"...<strong>Testing</strong>\n<span class="math"...</p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p><strong>Testing</strong>\n<span class="math notranslate">monospace</span></p>\n\n\n</div>'
**********************************************************************
File "src/sage/misc/sphinxify.py", line 53, in sage.misc.sphinxify.sphinxify
Failed example:
    sphinxify('`x=y`')
Expected:
    '...<div class="docstring">\n    \n  <p><span class="math">x=y</span></p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p><span class="math notranslate">x=y</span></p>\n\n\n</div>'
**********************************************************************

comment:12 Changed 5 years ago by rws

Status: needs_infoneeds_work

comment:13 Changed 5 years ago by jdemeyer

I did not consider that Sphinx might affect doctests too.

comment:14 Changed 5 years ago by git

Commit: 4fc721aa2203e9c4ff34df1acedcd70f443b96b97419e0246230594ebfd5e7a2fe6b80d67abfc98a

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

7419e02Fix sphinxify doctests

comment:15 Changed 5 years ago by jdemeyer

Status: needs_workneeds_review

comment:16 in reply to:  13 Changed 5 years ago by embray

Replying to jdemeyer:

I did not consider that Sphinx might affect doctests too.

It definitely can :) There are several places in Python 3 where I've had to make sure to ignore some python 3 specific DeprecationWarnings from Sphinx at import time or else it broke some tests. So I'm not too surprised that a Sphinx upgrade might also affect some docbuild-related tests.

comment:17 Changed 5 years ago by rws

Reviewers: Ralf Stephan
Status: needs_reviewpositive_review

It would be great if patchbot could test "unsafe" tickets but I remember about difficulties.

comment:18 in reply to:  17 ; Changed 5 years ago by embray

Replying to rws:

It would be great if patchbot could test "unsafe" tickets but I remember about difficulties.

I already have a patch for patchbot that will allow it to test tickets with new packages. It's been running on my Cygwin patchbot for a while now, but it does occasionally run into hiccups requiring me to manually restart the patchbot (after cleaning things up a bit).

I especially am not 100% sure why this happens sometimes, since the patchbot creates a copy of SAGE_LOCAL for "unsafe" tickets before doing much else.

comment:19 in reply to:  18 Changed 5 years ago by jdemeyer

Speaking of "unsafe" tickets, I'm not convinced that "unsafe" tickets are on average less safe than "safe" tickets. We certainly have had "safe" tickets messing up a patchbot installation.

comment:20 Changed 5 years ago by embray

Yes, it's a pretty flimsy distinction.

comment:21 in reply to:  18 Changed 5 years ago by rws

Replying to embray:

I especially am not 100% sure why this happens sometimes, since the patchbot creates a copy of SAGE_LOCAL for "unsafe" tickets before doing much else.

Please note https://github.com/sagemath/sage-patchbot/issues/34

comment:22 Changed 5 years ago by vbraun

Status: positive_reviewneeds_work

PDF docs don't build...

comment:23 Changed 5 years ago by jdemeyer

Description: modified (diff)
Milestone: sage-8.2sage-8.3
Summary: Upgrade to Sphinx 1.7.1Upgrade to Sphinx 1.7.4

comment:24 Changed 5 years ago by jdemeyer

Dependencies: #24972

comment:25 Changed 5 years ago by git

Commit: 7419e0246230594ebfd5e7a2fe6b80d67abfc98ae49afe3056470ababad81d34fe0e65c43e0bebec

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

5bbd500Upgrade to Sphinx 1.7.4
e49afe3Fix sphinxify doctests

comment:26 Changed 5 years ago by git

Commit: e49afe3056470ababad81d34fe0e65c43e0bebec5741161e15e78db9be8ef2e8a6c30af740893088

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

5741161Run TeX for docbuild non-interactively

comment:27 Changed 5 years ago by jdemeyer

Description: modified (diff)
Report Upstream: N/AReported upstream. No feedback yet.

comment:28 in reply to:  22 Changed 5 years ago by jdemeyer

Replying to vbraun:

PDF docs don't build...

Indeed... unfortunately it's a genuine problem with Sphinx. I reported it at https://github.com/sphinx-doc/sphinx/issues/4978

comment:29 Changed 5 years ago by jdemeyer

Description: modified (diff)
Report Upstream: Reported upstream. No feedback yet.Completely fixed; Fix reported upstream
Summary: Upgrade to Sphinx 1.7.4Upgrade to Sphinx 1.7.5

comment:30 Changed 5 years ago by git

Commit: 5741161e15e78db9be8ef2e8a6c30af740893088f4ca3d2d14d73770250c99c962ea958effca3af5

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

3e86444Run TeX for docbuild non-interactively
ae63489Upgrade to Sphinx 1.7.5
f4ca3d2Fix sphinxify doctests

comment:31 Changed 5 years ago by jdemeyer

Description: modified (diff)
Report Upstream: Completely fixed; Fix reported upstreamReported upstream. No feedback yet.

comment:32 Changed 5 years ago by slelievre

The Sphinx problem with Brazilian is fixed [0] but now there is a Sphinx problem with Russian [1].

comment:33 Changed 5 years ago by git

Commit: f4ca3d2d14d73770250c99c962ea958effca3af5867372ce1c932870a9d9e4e5e0454a710f175072

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

3e86444Run TeX for docbuild non-interactively
7842049Upgrade to Sphinx 1.7.5
867372cFix sphinxify doctests

comment:34 Changed 5 years ago by jdemeyer

Report Upstream: Reported upstream. No feedback yet.Fixed upstream, but not in a stable release.
Status: needs_workneeds_review

This took several iterations to get right, but now the PDF docs build for me.

comment:35 Changed 5 years ago by jdemeyer

Description: modified (diff)

comment:36 Changed 5 years ago by fbissey

Upstream greatly simplified the patch for PR5400 but I'll admit I am unclear if it can be used alone or need some other PR to go with it.

comment:37 Changed 5 years ago by jdemeyer

Are you referring to https://github.com/sphinx-doc/sphinx/pull/5045?

Anyway, the current branch seems to work so I'd rather keep it.

comment:38 in reply to:  37 ; Changed 5 years ago by fbissey

Reviewers: Ralf StephanRalf Stephan, François Bissey
Status: needs_reviewpositive_review

Replying to jdemeyer:

Are you referring to https://github.com/sphinx-doc/sphinx/pull/5045?

Anyway, the current branch seems to work so I'd rather keep it.

No. 5040 has two commits. Your current patch is the first one. It is described as a temporary solution while the second commit is more "future proofing". But indeed the first commit you are using is supposed to work. Let's send this to the bots as is.

comment:39 in reply to:  38 Changed 5 years ago by jdemeyer

Replying to fbissey:

Your current patch is the first one.

It actually contains both commits.

comment:40 Changed 5 years ago by fbissey

Cumulative patches on the same files confuse me no end.

comment:41 Changed 5 years ago by vbraun

Status: positive_reviewneeds_work
sage -t --long src/sage/misc/sphinxify.py  # 2 doctests failed
sage -t --long local/lib/python2.7/site-packages/sagenb/misc/sphinxify.py  # 2 doctests failed

comment:42 Changed 5 years ago by fbissey

Cc: dimpase added

Trivial but we should have seen it in reviews

fbissey@moonloop ~ $ sage -t --long /usr/lib64/python2.7/site-packages/sage/misc/sphinxify.py 
too many failed tests, not using stored timings
Running doctests with ID 2018-06-10-20-16-29-93e55012.
Using --optional=optional,sage
Doctesting 1 file.
sage -t --long /usr/lib64/python2.7/site-packages/sage/misc/sphinxify.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/misc/sphinxify.py", line 51, in sage.misc.sphinxify.sphinxify
Failed example:
    sphinxify('**Testing**\n`monospace`')
Expected:
    '<div class="docstring"...<strong>Testing</strong>\n<span class="math notranslate"...</p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p><strong>Testing</strong>\n<span class="math notranslate nohighlight">monospace</span></p>\n\n\n</div>'
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/misc/sphinxify.py", line 53, in sage.misc.sphinxify.sphinxify
Failed example:
    sphinxify('`x=y`')
Expected:
    '<div class="docstring">\n    \n  <p><span class="math notranslate">x=y</span></p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p><span class="math notranslate nohighlight">x=y</span></p>\n\n\n</div>'
**********************************************************************
1 item had failures:
   2 of  10 in sage.misc.sphinxify.sphinxify
    [9 tests, 2 failures, 1.02 s]
----------------------------------------------------------------------
sage -t --long /usr/lib64/python2.7/site-packages/sage/misc/sphinxify.py  # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 1.0 seconds
    cpu time: 1.0 seconds
    cumulative wall time: 1.0 seconds
fbissey@moonloop ~ $ sage -t --long /usr/lib64/python2.7/site-packages/sagenb/misc/sphinxify.py 
too many failed tests, not using stored timings
Running doctests with ID 2018-06-10-20-17-44-197b0e5d.
Using --optional=optional,sage
Doctesting 1 file.
sage -t --long /usr/lib64/python2.7/site-packages/sagenb/misc/sphinxify.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sagenb/misc/sphinxify.py", line 68, in sagenb.misc.sphinxify.sphinxify
Failed example:
    sphinxify('**Testing**\n`monospace`')
Expected:
    '...<div class="docstring"...<strong>Testing</strong>\n<span class="math"...</p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p><strong>Testing</strong>\n<span class="math notranslate nohighlight">monospace</span></p>\n\n\n</div>'
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sagenb/misc/sphinxify.py", line 70, in sagenb.misc.sphinxify.sphinxify
Failed example:
    sphinxify('`x=y`')
Expected:
    '...<div class="docstring">\n    \n  <p><span class="math">x=y</span></p>\n\n\n</div>'
Got:
    '<div class="docstring">\n    \n  <p><span class="math notranslate nohighlight">x=y</span></p>\n\n\n</div>'
**********************************************************************
1 item had failures:
   2 of  10 in sagenb.misc.sphinxify.sphinxify
    [14 tests, 2 failures, 0.74 s]
----------------------------------------------------------------------
sage -t --long /usr/lib64/python2.7/site-packages/sagenb/misc/sphinxify.py  # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 1.2 seconds
    cpu time: 0.7 seconds
    cumulative wall time: 0.7 seconds

That unfortunately requires a new version of sagenb. @Dima could we get a 1.0.3 for this.

comment:43 Changed 5 years ago by dimpase

OK, so that's just doctests in sagenb, right? No problem, I'll fix these. https://github.com/sagemath/sagenb/issues/445

comment:44 in reply to:  42 Changed 5 years ago by jdemeyer

Replying to fbissey:

Trivial but we should have seen it in reviews

Right. I just did not think about running doctests after fixing the PDF documentation stuff.

comment:45 in reply to:  43 Changed 5 years ago by fbissey

Replying to dimpase:

OK, so that's just doctests in sagenb, right? No problem, I'll fix these. https://github.com/sagemath/sagenb/issues/445

The commit works for me. We just have to reproduce it in sage's sphinxify.py.

comment:46 Changed 5 years ago by dimpase

here is the new sagenb https://github.com/sagemath/sagenb/releases/download/1.0.3/sagenb-1.0.3.tar.bz2

Could you bump up the version in build/pkgs/sagenb/ etc to include it on this ticket?

comment:47 in reply to:  46 ; Changed 5 years ago by fbissey

Replying to dimpase:

here is the new sagenb https://github.com/sagemath/sagenb/releases/download/1.0.3/sagenb-1.0.3.tar.bz2

Could you bump up the version in build/pkgs/sagenb/ etc to include it on this ticket?

Definitely. I am leaving it to Jeroen as the ticket author, unless he indicates he is too busy to handle it in which case I will step up.

comment:48 in reply to:  47 Changed 5 years ago by jdemeyer

Replying to fbissey:

I am leaving it to Jeroen as the ticket author, unless he indicates he is too busy to handle it in which case I will step up.

It's literally my job to do this...

comment:49 Changed 5 years ago by jdemeyer

Description: modified (diff)

comment:50 Changed 5 years ago by git

Commit: 867372ce1c932870a9d9e4e5e0454a710f175072e398cf8db43373b1b1c2f1974404d33e9e164d63

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

e664816Run TeX for docbuild non-interactively
1a073deUpgrade to Sphinx 1.7.5
4c0bc88Upgrade sagenb
e398cf8Fix sphinxify doctests

comment:51 Changed 5 years ago by jdemeyer

Status: needs_workneeds_review

comment:52 Changed 5 years ago by fbissey

Status: needs_reviewpositive_review

Works for me.

comment:53 Changed 5 years ago by chapoton

Damn. Could we get rid of the "import __builtin__" in sagenb/misc/sphinxify.py and sagenb/misc/support.py ??

This is breaking the compatibility of sagenb with python3...

comment:54 in reply to:  53 ; Changed 5 years ago by jdemeyer

Replying to chapoton:

Damn. Could we get rid of the "import __builtin__" in sagenb/misc/sphinxify.py and sagenb/misc/support.py ??

Perhaps, but not in this ticket please.

comment:55 in reply to:  54 Changed 5 years ago by fbissey

Replying to jdemeyer:

Replying to chapoton:

Damn. Could we get rid of the "import __builtin__" in sagenb/misc/sphinxify.py and sagenb/misc/support.py ??

Perhaps, but not in this ticket please.

Totally agree with Jeroen, another ticket please.

That being said, historically we have migrated sphinxify.py from sagenb into sage to reduce dependency on sagenb (that's what I remember at least). If you want to work on that, I would really suggest to try eliminating the sphinxify.py of sagenb and use sage's one instead.

comment:56 Changed 5 years ago by vbraun

Status: positive_reviewneeds_work
File "src/doc/common/conf.py", line 627, in doc.common.conf.call_intersphinx
Failed example:
    for line in open(thematic_index).readlines():
        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 ...)"><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)"><span>Introduction to the -adics</span></a></li>
**********************************************************************
1 item had failures:
   1 of   4 in doc.common.conf.call_intersphinx
    [3 tests, 1 failure, 0.61 s]

comment:57 Changed 5 years ago by fbissey

I have seen it too in the vbraun branch and it confused me quite a bit because I don't remember seeing it the first time round but it should have been there. It seems like before this ticket the title field ends up with v as in title="(in Sage Reference Manual: p-Adics v) not just for this particular one that's tested but for all of them. After this ticket it is reduced to just title="(in Sage Reference Manual: p-Adics) and the v has disappeared everywhere.

comment:58 Changed 5 years ago by jdemeyer

It's supposed to say v8.3.beta5 so that's a regression somehow.

comment:59 Changed 5 years ago by jdemeyer

Dependencies: #25570
Status: needs_workpositive_review

I opened #25570 for the version number regression.

comment:60 Changed 5 years ago by git

Commit: e398cf8db43373b1b1c2f1974404d33e9e164d634e066baffdce49ccc9bcc402b1ff534022b5f355
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

3b57916Fix version number in built documentation
b321995Run TeX for docbuild non-interactively
8a71e29Upgrade to Sphinx 1.7.5
a22da72Upgrade sagenb
4e066baFix sphinxify doctests

comment:61 Changed 5 years ago by jdemeyer

Status: needs_reviewpositive_review

comment:62 Changed 5 years ago by vbraun

Branch: u/jdemeyer/upgrade_to_sphinx_1_7_14e066baffdce49ccc9bcc402b1ff534022b5f355
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.