Change History (62)

comment:1 Changed 2 years ago by fbissey

  • Cc fbissey added

comment:2 Changed 2 years ago by jdemeyer

  • Dependencies set to #24972

comment:3 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 2 years ago by jdemeyer

  • Branch set to u/jdemeyer/upgrade_to_sphinx_1_7_1

comment:5 Changed 2 years ago by jdemeyer

  • Commit set to 41f114ec76d1e97701f77e0b197615faab308a10
  • Description modified (diff)

New commits:

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

comment:6 Changed 2 years ago by git

  • Commit changed from 41f114ec76d1e97701f77e0b197615faab308a10 to 83682b257e42815d8608db5440f63983ad723088

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 2 years ago by git

  • Commit changed from 83682b257e42815d8608db5440f63983ad723088 to 4fc721aa2203e9c4ff34df1acedcd70f443b96b9

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 2 years ago by jdemeyer

  • Status changed from new to needs_review

comment:9 follow-up: Changed 2 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 2 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 2 years ago by rws

  • Status changed from needs_review to 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 2 years ago by rws

  • Status changed from needs_info to needs_work

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

I did not consider that Sphinx might affect doctests too.

comment:14 Changed 2 years ago by git

  • Commit changed from 4fc721aa2203e9c4ff34df1acedcd70f443b96b9 to 7419e0246230594ebfd5e7a2fe6b80d67abfc98a

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

7419e02Fix sphinxify doctests

comment:15 Changed 2 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:16 in reply to: ↑ 13 Changed 2 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 follow-up: Changed 2 years ago by rws

  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

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

comment:18 in reply to: ↑ 17 ; follow-ups: Changed 2 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 2 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 2 years ago by embray

Yes, it's a pretty flimsy distinction.

comment:21 in reply to: ↑ 18 Changed 2 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 follow-up: Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs don't build...

comment:23 Changed 23 months ago by jdemeyer

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

comment:24 Changed 23 months ago by jdemeyer

  • Dependencies #24972 deleted

comment:25 Changed 23 months ago by git

  • Commit changed from 7419e0246230594ebfd5e7a2fe6b80d67abfc98a to e49afe3056470ababad81d34fe0e65c43e0bebec

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 23 months ago by git

  • Commit changed from e49afe3056470ababad81d34fe0e65c43e0bebec to 5741161e15e78db9be8ef2e8a6c30af740893088

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

5741161Run TeX for docbuild non-interactively

comment:27 Changed 23 months ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:28 in reply to: ↑ 22 Changed 23 months 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 22 months ago by jdemeyer

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

comment:30 Changed 22 months ago by git

  • Commit changed from 5741161e15e78db9be8ef2e8a6c30af740893088 to f4ca3d2d14d73770250c99c962ea958effca3af5

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 22 months ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from Completely fixed; Fix reported upstream to Reported upstream. No feedback yet.

comment:32 Changed 22 months ago by slelievre

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

comment:33 Changed 22 months ago by git

  • Commit changed from f4ca3d2d14d73770250c99c962ea958effca3af5 to 867372ce1c932870a9d9e4e5e0454a710f175072

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 22 months ago by jdemeyer

  • Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
  • Status changed from needs_work to needs_review

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

comment:35 Changed 22 months ago by jdemeyer

  • Description modified (diff)

comment:36 Changed 22 months 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 follow-up: Changed 22 months 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 ; follow-up: Changed 22 months ago by fbissey

  • Reviewers changed from Ralf Stephan to Ralf Stephan, François Bissey
  • Status changed from needs_review to 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 in reply to: ↑ 38 Changed 22 months ago by jdemeyer

Replying to fbissey:

Your current patch is the first one.

It actually contains both commits.

comment:40 Changed 22 months ago by fbissey

Cumulative patches on the same files confuse me no end.

comment:41 Changed 22 months ago by vbraun

  • Status changed from positive_review to 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: Changed 22 months 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 follow-up: Changed 22 months 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 22 months 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 22 months 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 follow-up: Changed 22 months 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 ; follow-up: Changed 22 months 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 22 months 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 22 months ago by jdemeyer

  • Description modified (diff)

comment:50 Changed 22 months ago by git

  • Commit changed from 867372ce1c932870a9d9e4e5e0454a710f175072 to e398cf8db43373b1b1c2f1974404d33e9e164d63

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 22 months ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:52 Changed 22 months ago by fbissey

  • Status changed from needs_review to positive_review

Works for me.

comment:53 follow-up: Changed 22 months 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 ; follow-up: Changed 22 months 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 22 months 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 22 months ago by vbraun

  • Status changed from positive_review to 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 22 months 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 22 months ago by jdemeyer

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

comment:59 Changed 22 months ago by jdemeyer

  • Dependencies set to #25570
  • Status changed from needs_work to positive_review

I opened #25570 for the version number regression.

comment:60 Changed 22 months ago by git

  • Commit changed from e398cf8db43373b1b1c2f1974404d33e9e164d63 to 4e066baffdce49ccc9bcc402b1ff534022b5f355
  • Status changed from positive_review to 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:

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 22 months ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:62 Changed 22 months ago by vbraun

  • Branch changed from u/jdemeyer/upgrade_to_sphinx_1_7_1 to 4e066baffdce49ccc9bcc402b1ff534022b5f355
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.