Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build with sphinx 7 #35658

Merged
merged 6 commits into from
Jun 3, 2023
Merged

Fix build with sphinx 7 #35658

merged 6 commits into from
Jun 3, 2023

Conversation

antonio-rojas
Copy link
Contributor

Replace or remove use of deprecated stuff

Tried to keep it compatible with older sphinx, but needs testing

Replace or remove use of deprecated stuff
@kiwifb
Copy link
Member

kiwifb commented May 21, 2023

Thanks for this. I looked at things when sphinx 6 came out and just gave up. sage_autodoc.py needs to die one day soon.

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2023

Tried to keep it compatible with older sphinx, but needs testing

Testing is most important in this business of maintaining the documentation stuff, I think. Did you at least check the built documentation has no regressions?

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2023

The built documents look good to me.

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2023

Thanks. But now we have

[sagemath_doc_html-none] [reference]   File "/__w/sage/sage/src/sage_docbuild/ext/sage_autodoc.py", line 651, in Documenter
[sagemath_doc_html-none] [reference]     def get_object_members(self, want_all: bool) -> tuple[bool, ObjectMembers]:
[sagemath_doc_html-none] [reference] TypeError: 'type' object is not subscriptable

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2023

It seems "tuple" type annotation is only introduced in python 3.9, and "Tuple" is deprecated since python 3.9.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 23, 2023

Per https://doc.sagemath.org/html/en/developer/coding_in_python.html#python-language-standard, this file should use from __future__ import annotations

@antonio-rojas
Copy link
Contributor Author

It seems "tuple" type annotation is only introduced in python 3.9, and "Tuple" is deprecated since python 3.9.

Changed it to Tuple for consistency with the other type annotations in this file, they can all be ported together at a later time.

@github-actions
Copy link

Documentation preview for this PR (built with commit d93dc08) is ready! 🎉

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kiwifb
Copy link
Member

kiwifb commented May 24, 2023

I have successfully build the doc in sage-on-gentoo with both sphinx 5 and sphinx 7. Definitely happy with the PR.

@tobiasdiez
Copy link
Contributor

@kiwifb
Copy link
Member

kiwifb commented Jun 2, 2023

Are all build issues with sphinx 7 fixed by this PR? If yes, can we remove the version constraints in https://github.com/sagemath/sage/blob/develop/build/pkgs/sphinx/install-requires.txt and https://github.com/sagemath/sage/blob/develop/build/pkgs/sphinx/distros/conda.txt?

I haven't encountered any issues with sphinx 7. So, I would say you should go ahead with that, but in a separate PR since this one is being merged.

@vbraun vbraun merged commit cacd9a8 into sagemath:develop Jun 3, 2023
@mkoeppe mkoeppe added this to the sage-10.1 milestone Jun 3, 2023
@dimpase
Copy link
Member

dimpase commented Jun 4, 2023

We definitely should update our sphinx, as it blocks conda (they need >= 6.0 for some Sage deps) @isuruf

@tobiasdiez
Copy link
Contributor

Are all build issues with sphinx 7 fixed by this PR? If yes, can we remove the version constraints in https://github.com/sagemath/sage/blob/develop/build/pkgs/sphinx/install-requires.txt and https://github.com/sagemath/sage/blob/develop/build/pkgs/sphinx/distros/conda.txt?

I haven't encountered any issues with sphinx 7. So, I would say you should go ahead with that, but in a separate PR since this one is being merged.

Done now at #35845 (ready for review).

@antonio-rojas antonio-rojas deleted the sphinx-7 branch July 2, 2023 10:36
@dimpase
Copy link
Member

dimpase commented Sep 16, 2023

I seem to have a problem with sphinx 7.2.6:

[sagemath_doc_html-none] [plotting ] /home/dimpase/work/software/sage/src/sage/plot/contour_plot.py:docstring of sage.plot.contour_plot.region_plot:1: WARNING: could not parse arglist ("f, xrange, yrange, plot_points, incol, outcol, bordercol, borderstyle, borderwidth, alpha, plot_points=100, incol='blue', outcol=None, bordercol=None, borderstyle=None, borderwidth=None, frame=False, axes=True, legend_label=None, aspect_ratio=1, alpha=1, **options"): duplicate parameter name: 'plot_points'

has anyone seen this? Looks like it complains about @options(plot_points=100,...
decorator on region_plot.

@antonio-rojas
Copy link
Contributor Author

has anyone seen this? Looks like it complains about @options(plot_points=100,... decorator on region_plot.

Yes. For now I'm whitelisting that warning in sage_docbuild/sphinxbuild.py, since I don't really understand decorators.

@kiwifb
Copy link
Member

kiwifb commented Sep 16, 2023

I have seen trouble with both 7.1 and 7.2. In sage-on-gentoo anything sphinx 7.1 and over breaks the build last time I checked. See cschwan/sage-on-gentoo#753

@dimpase
Copy link
Member

dimpase commented Sep 16, 2023

I can confirm that sphinx 7.0.0 (with docutils 0.19) still works for me (in a pyenv and Python 3.11.5 there, in conjunction with trying out #36256), but 7.1.0 (with docutils 0.20) does not.

@antonio-rojas
Copy link
Contributor Author

I have seen trouble with both 7.1 and 7.2. In sage-on-gentoo anything sphinx 7.1 and over breaks the build last time I checked. See cschwan/sage-on-gentoo#753

I can't reproduce this with 7.2.6

@dimpase
Copy link
Member

dimpase commented Sep 16, 2023

I'll try from scratch, again.

@dimpase
Copy link
Member

dimpase commented Sep 16, 2023

but what I saw was a different error

@kiwifb
Copy link
Member

kiwifb commented Sep 16, 2023

Could be a combination of sphinx with something else that fails. Right now furo-2023-8.19 leads to truncated html documentation for me as well. The documentation build is just in a bad place right now.

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 16, 2023

Please open a new issue.

dimpase added a commit to dimpase/sage that referenced this pull request Sep 18, 2023
dimpase added a commit to dimpase/sage that referenced this pull request Sep 25, 2023
dimpase added a commit to dimpase/sage that referenced this pull request Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants