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: |
Description (last modified by )
Tarballs:
Upstream bugs:
- https://github.com/sphinx-doc/sphinx/issues/4978 (fixed in release 1.7.5)
- https://github.com/sphinx-doc/sphinx/issues/5040 (fixed in master; patch added to Sage)
Change History (62)
comment:1 Changed 5 years ago by
Cc: | fbissey added |
---|
comment:2 Changed 5 years ago by
Dependencies: | → #24972 |
---|
comment:3 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 5 years ago by
Branch: | → u/jdemeyer/upgrade_to_sphinx_1_7_1 |
---|
comment:5 Changed 5 years ago by
Commit: | → 41f114ec76d1e97701f77e0b197615faab308a10 |
---|---|
Description: | modified (diff) |
comment:6 Changed 5 years ago by
Commit: | 41f114ec76d1e97701f77e0b197615faab308a10 → 83682b257e42815d8608db5440f63983ad723088 |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
83682b2 | Upgrade to Sphinx 1.7.1
|
comment:7 Changed 5 years ago by
Commit: | 83682b257e42815d8608db5440f63983ad723088 → 4fc721aa2203e9c4ff34df1acedcd70f443b96b9 |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4fc721a | Upgrade to Sphinx 1.7.1
|
comment:8 Changed 5 years ago by
Status: | new → needs_review |
---|
comment:9 follow-up: 10 Changed 5 years ago by
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 Changed 5 years ago by
comment:11 Changed 5 years ago by
Status: | needs_review → needs_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
Status: | needs_info → needs_work |
---|
comment:13 follow-up: 16 Changed 5 years ago by
I did not consider that Sphinx might affect doctests too.
comment:14 Changed 5 years ago by
Commit: | 4fc721aa2203e9c4ff34df1acedcd70f443b96b9 → 7419e0246230594ebfd5e7a2fe6b80d67abfc98a |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
7419e02 | Fix sphinxify doctests
|
comment:15 Changed 5 years ago by
Status: | needs_work → needs_review |
---|
comment:16 Changed 5 years ago by
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 DeprecationWarning
s 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 follow-up: 18 Changed 5 years ago by
Reviewers: | → Ralf Stephan |
---|---|
Status: | needs_review → positive_review |
It would be great if patchbot could test "unsafe" tickets but I remember about difficulties.
comment:18 follow-ups: 19 21 Changed 5 years ago by
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 Changed 5 years ago by
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:21 Changed 5 years ago by
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 follow-up: 28 Changed 5 years ago by
Status: | positive_review → needs_work |
---|
PDF docs don't build...
comment:23 Changed 5 years ago by
Description: | modified (diff) |
---|---|
Milestone: | sage-8.2 → sage-8.3 |
Summary: | Upgrade to Sphinx 1.7.1 → Upgrade to Sphinx 1.7.4 |
comment:24 Changed 5 years ago by
Dependencies: | #24972 |
---|
comment:25 Changed 5 years ago by
Commit: | 7419e0246230594ebfd5e7a2fe6b80d67abfc98a → e49afe3056470ababad81d34fe0e65c43e0bebec |
---|
comment:26 Changed 5 years ago by
Commit: | e49afe3056470ababad81d34fe0e65c43e0bebec → 5741161e15e78db9be8ef2e8a6c30af740893088 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
5741161 | Run TeX for docbuild non-interactively
|
comment:27 Changed 5 years ago by
Description: | modified (diff) |
---|---|
Report Upstream: | N/A → Reported upstream. No feedback yet. |
comment:28 Changed 5 years ago by
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
Description: | modified (diff) |
---|---|
Report Upstream: | Reported upstream. No feedback yet. → Completely fixed; Fix reported upstream |
Summary: | Upgrade to Sphinx 1.7.4 → Upgrade to Sphinx 1.7.5 |
comment:30 Changed 5 years ago by
Commit: | 5741161e15e78db9be8ef2e8a6c30af740893088 → f4ca3d2d14d73770250c99c962ea958effca3af5 |
---|
comment:31 Changed 5 years ago by
Description: | modified (diff) |
---|---|
Report Upstream: | Completely fixed; Fix reported upstream → Reported upstream. No feedback yet. |
comment:32 Changed 5 years ago by
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
Commit: | f4ca3d2d14d73770250c99c962ea958effca3af5 → 867372ce1c932870a9d9e4e5e0454a710f175072 |
---|
comment:34 Changed 5 years ago by
Report Upstream: | Reported upstream. No feedback yet. → Fixed upstream, but not in a stable release. |
---|---|
Status: | needs_work → needs_review |
This took several iterations to get right, but now the PDF docs build for me.
comment:35 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:36 Changed 5 years ago by
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 follow-up: 38 Changed 5 years ago by
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 follow-up: 39 Changed 5 years ago by
Reviewers: | Ralf Stephan → Ralf Stephan, François Bissey |
---|---|
Status: | needs_review → positive_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 Changed 5 years ago by
comment:41 Changed 5 years ago by
Status: | positive_review → needs_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 follow-up: 44 Changed 5 years ago by
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 follow-up: 45 Changed 5 years ago by
OK, so that's just doctests in sagenb, right? No problem, I'll fix these. https://github.com/sagemath/sagenb/issues/445
comment:44 Changed 5 years ago by
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 Changed 5 years ago by
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 follow-up: 47 Changed 5 years ago by
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 follow-up: 48 Changed 5 years ago by
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 Changed 5 years ago by
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
Description: | modified (diff) |
---|
comment:50 Changed 5 years ago by
Commit: | 867372ce1c932870a9d9e4e5e0454a710f175072 → e398cf8db43373b1b1c2f1974404d33e9e164d63 |
---|
comment:51 Changed 5 years ago by
Status: | needs_work → needs_review |
---|
comment:53 follow-up: 54 Changed 5 years ago by
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 follow-up: 55 Changed 5 years ago by
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 Changed 5 years ago by
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
Status: | positive_review → needs_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
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:59 Changed 5 years ago by
Dependencies: | → #25570 |
---|---|
Status: | needs_work → positive_review |
I opened #25570 for the version number regression.
comment:60 Changed 5 years ago by
Commit: | e398cf8db43373b1b1c2f1974404d33e9e164d63 → 4e066baffdce49ccc9bcc402b1ff534022b5f355 |
---|---|
Status: | positive_review → needs_review |
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:
3b57916 | Fix version number in built documentation
|
b321995 | Run TeX for docbuild non-interactively
|
8a71e29 | Upgrade to Sphinx 1.7.5
|
a22da72 | Upgrade sagenb
|
4e066ba | Fix sphinxify doctests
|
comment:61 Changed 5 years ago by
Status: | needs_review → positive_review |
---|
comment:62 Changed 5 years ago by
Branch: | u/jdemeyer/upgrade_to_sphinx_1_7_1 → 4e066baffdce49ccc9bcc402b1ff534022b5f355 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
New commits:
added sphinxcontrib_websupport
Minor fixes to sphinxcontrib_websupport
Upgrade to Sphinx 1.7.1