Update to sphinx 5.2 #34615

antonio-rojas · 2022-09-29T19:24:08Z

New changes in sphinx requires new attributes to be defined. Supporting this change introduces non fatal error and warnings when building with older version of sphinx.

Therefore we think it is time to upgrade sphinx in sage.

The ticket update

sphinx to 5.2.3
sphinxcontrib_websupport to 1.2.4 (no other sphinxcontrib package seem out of date)
imagesize to 1.4.1 (required dependency)

CC: @kiwifb

Component: build

Author: Antonio Rojas, François Bissey

Branch: 8f8af65

Reviewer: Kwankyu Lee

Issue created by migration from https://trac.sagemath.org/ticket/34615

The text was updated successfully, but these errors were encountered:

antonio-rojas · 2022-09-29T19:24:44Z

Branch: u/arojas/fix_build_with_sphinx_5_2

sagetrac-git · 2022-09-29T19:29:14Z

Commit: c2e610c

sagetrac-git · 2022-09-29T19:29:14Z

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

`c2e610c`	`Fix build with sphinx 5.2`

antonio-rojas · 2022-09-29T19:31:11Z

Author: Antonio Rojas

kiwifb · 2022-09-29T19:47:50Z

comment:4

I haven't been pushed to 5.2 yet, so I haven't seen it. Although I could have had reports. This is quite different from the previous fix we pushed. Was it supposed to be implicit in older version of sphinx.

antonio-rojas · 2022-09-29T19:58:16Z

comment:5

No, this is a new attribute in 5.2. That's why I was surprised this still works with older sphinx

sphinx-doc/sphinx@7da60f2

antonio-rojas · 2022-09-29T20:03:09Z

comment:6

Ah, no. It does throw errors with older sphinx, although docs still seem to build fine.

kiwifb · 2022-09-29T20:05:12Z

comment:7

I suggest we take the opportunity to upgrade sphinx in sage itself then. I didn't do it for 5.1 and that may have been lazy on my part :)

sagetrac-git · 2022-09-29T21:00:58Z

Changed commit from c2e610c to 0be8a8f

sagetrac-git · 2022-09-29T21:00:58Z

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

`0be8a8f`	`Update sphinx to 5.2`

kiwifb · 2022-09-29T21:15:48Z

comment:9

I am busy this morning but if you don't do more, I'll look at the sphinx contribution packages that may need updating or- be added to sage.

antonio-rojas · 2022-09-30T06:00:23Z

comment:10

Everything builds fine and docs look OK

kiwifb · 2022-10-05T09:25:18Z

comment:11

After looking more closely, only sphinxcontrib_websupport would need a version bump.

kiwifb · 2022-10-05T09:59:31Z

Changed commit from 0be8a8f to 4a8c2b2

kiwifb · 2022-10-05T09:59:31Z

Changed author from Antonio Rojas to Antonio Rojas, François Bissey

kiwifb · 2022-10-05T09:59:31Z

Changed branch from u/arojas/fix_build_with_sphinx_5_2 to u/fbissey/sphinx5_2

kiwifb · 2022-10-05T09:59:31Z

New commits:

`4a8c2b2`	`update sphinxcontrib_websupport to latest`

kwankyu · 2022-10-07T13:42:11Z

comment:13

On macos, with the ticket branch merged to the develop branch, I encountered

[sagemath_doc_html-none]     raise DistributionNotFound(req, requirers)
[sagemath_doc_html-none] pkg_resources.DistributionNotFound: The 'imagesize>=1.3'
 distribution was not found and is required by sphinx

kwankyu · 2022-10-07T13:47:05Z

comment:14

Upgrading to imagesize-1.4.1 the latest gets it rolling.

kwankyu · 2022-10-07T20:28:05Z

comment:15

Replying to Kwankyu Lee:

Upgrading to imagesize-1.4.1 the latest gets it rolling.

It ended up successfully, and the built documents look good.

kiwifb · 2022-10-09T21:48:19Z

comment:17

You are right, the version of imagesize was way to low in sage (1.2). Updating now.

sagetrac-git · 2022-10-09T21:58:18Z

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

`5cf5836`	`update imagesize to 1.4.1. Minimum required for sphinx 5.2 is 1.3.`
`8f8af65`	`update sphinx to 5.2.3`

sagetrac-git · 2022-10-09T21:58:18Z

Changed commit from 4a8c2b2 to 8f8af65

kiwifb · 2022-10-09T22:00:33Z

comment:19

Updated imagesize and pushed sphinx to 5.2.3.

kwankyu · 2022-10-10T02:09:19Z

comment:20

Thanks.

It works well and looks good to me.

kwankyu · 2022-10-10T02:09:19Z

Reviewer: Kwankyu Lee

vbraun · 2022-10-16T22:15:42Z

Changed branch from u/fbissey/sphinx5_2 to 8f8af65

kwankyu · 2022-10-24T06:18:54Z

comment:22

This upgrade seems to have introduced an unpleasant change of doc appearance. Compare

https://8f8af65e54d3a9962cfab40f15dc23f4e955b43f--sagemath-tobias.netlify.app/reference/curves/index.html

with

https://doc.sagemath.org/html/en/reference/curves/index.html

See all the second-level headings. Those weren't before.

kwankyu · 2022-10-24T06:18:54Z

Changed commit from 8f8af65 to none

kwankyu · 2022-10-25T01:31:05Z

comment:23

I suspect that the upgrade does not work well with sage's custom autodoc.

kiwifb · 2022-10-25T01:34:06Z

comment:24

Possibly, it is hardly news that we should get rid of it as quickly as we can. I wanted to ask whether you had an idea why there are subsection for say "plane conics" but not "curves"? When I tried to post yesterday, trac was in maintenance :(

kwankyu · 2022-10-25T02:32:12Z

comment:25

Replying to François Bissey:

Possibly, it is hardly news that we should get rid of it as quickly as we can.

I agree! But that seems a formidable task (unless we just leave the consequential "broken doc" at it is).

I wanted to ask whether you had an idea why there are subsection for say "plane conics" but not "curves"?

It is because of the maxdepth: 2. See

Plane conics
============

.. toctree::
   :maxdepth: 2

Hence it is normal. The real problem seems that definitions doc is regarded as content headings. I don't know if this is a feature of the upgraded sphinx or a problem of sage_autodoc...

kwankyu · 2022-10-25T04:00:20Z

comment:26

A simple and laborious solution would be to change all affected maxdepth: 2 to maxdepth: 1. This seems also a correct solution. Then we can avoid touching sage_autodoc :)

kwankyu · 2022-10-25T08:43:21Z

comment:27

Replying to Kwankyu Lee:

A simple and laborious solution would be to change all affected maxdepth: 2 to maxdepth: 1. This seems also a correct solution. Then we can avoid touching sage_autodoc :)

This certainly does not solve all problems. See the "Contents" on the right

https://8f8af65e54d3a9962cfab40f15dc23f4e955b43f--sagemath-tobias.netlify.app/reference/curves/sage/schemes/plane_conics/constructor.html

kiwifb · 2022-10-25T08:52:05Z

comment:28

I don't think that's bad. It is on a static right space while the content is just in a middle frame. This really looks like a standard template. One I have seen in other places.

Of course, if it is inconsistent with other pages, we have a problem.

kwankyu · 2022-10-31T04:57:22Z

comment:29

Replying to François Bissey:

I don't think that's bad. It is on a static right space while the content is just in a middle frame. This really looks like a standard template. One I have seen in other places.

Object definitions in TOC is a new standard feature of the upgraded Sphinx. But this changes our doc in an unintended way. This is fixed now in #34706. Needs review.

Sphinx, upgraded to 5.2.3 in sagemath#34615, builds the doc with object (classes, functions, ...) entries in toc (table of contents). We keep the object entries in TOC, but remove them from the main index pages. URL: https://trac.sagemath.org/34710 Reported by: klee Ticket author(s): Kwankyu Lee Reviewer(s): François Bissey

antonio-rojas added this to the sage-9.8 milestone Sep 29, 2022

antonio-rojas added the p: major / 3 label Sep 29, 2022

This comment has been minimized.

Sign in to view

antonio-rojas added t: enhancement labels Sep 29, 2022

antonio-rojas added s: needs work and removed s: needs review labels Sep 29, 2022

This comment has been minimized.

Sign in to view

kiwifb changed the title ~~Fix build with sphinx 5.2~~ Update to sphinx 5.2 Sep 29, 2022

antonio-rojas added s: needs review and removed s: needs work labels Sep 30, 2022

This comment has been minimized.

Sign in to view

kwankyu added s: positive review and removed s: needs review labels Oct 10, 2022

vbraun removed the s: positive review label Oct 16, 2022

vbraun closed this as completed in 7563675 Oct 16, 2022

vbraun mentioned this issue Oct 11, 2022

fix more W605 in rings #34645

Closed

This was referenced Jan 1, 2023

Remove object entries from TOC #34706

Closed

Keep object entries in TOC in short form #34710

Closed

Update to sphinx 5.2 #34615

Update to sphinx 5.2 #34615

Comments

antonio-rojas commented Sep 29, 2022

antonio-rojas commented Sep 29, 2022

sagetrac-git mannequin commented Sep 29, 2022

sagetrac-git mannequin commented Sep 29, 2022

This comment has been minimized.

antonio-rojas commented Sep 29, 2022

kiwifb commented Sep 29, 2022

antonio-rojas commented Sep 29, 2022

antonio-rojas commented Sep 29, 2022

kiwifb commented Sep 29, 2022

sagetrac-git mannequin commented Sep 29, 2022

sagetrac-git mannequin commented Sep 29, 2022

This comment has been minimized.

kiwifb commented Sep 29, 2022

antonio-rojas commented Sep 30, 2022

kiwifb commented Oct 5, 2022

This comment has been minimized.

kiwifb commented Oct 5, 2022

kiwifb commented Oct 5, 2022

kiwifb commented Oct 5, 2022

kiwifb commented Oct 5, 2022

kwankyu commented Oct 7, 2022

kwankyu commented Oct 7, 2022

kwankyu commented Oct 7, 2022

kiwifb commented Oct 9, 2022

sagetrac-git mannequin commented Oct 9, 2022

sagetrac-git mannequin commented Oct 9, 2022

kiwifb commented Oct 9, 2022

This comment has been minimized.

kwankyu commented Oct 10, 2022

kwankyu commented Oct 10, 2022

vbraun commented Oct 16, 2022

kwankyu commented Oct 24, 2022

kwankyu commented Oct 24, 2022

kwankyu commented Oct 25, 2022

kiwifb commented Oct 25, 2022

kwankyu commented Oct 25, 2022

kwankyu commented Oct 25, 2022

kwankyu commented Oct 25, 2022

kiwifb commented Oct 25, 2022

kwankyu commented Oct 31, 2022