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

rebase sage_autodoc with sphinx-8.2.3 and update sphinx to same. #39737

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

kiwifb
Copy link
Member

@kiwifb kiwifb commented Mar 18, 2025

Sphinx 8.2.3 is available and some people want to use it from their systems.
The current sage_autodoc is in sync with sphinx 8.1.3 and is not compatible with 8.2.3.
synchronizing with sphinx 8.2.3 also means we have to define intersphinx_resolve_self in sage_docbuild/conf.py otherwise the builds dies horribly.
Finally, synchronizing sage_autodoc with sphinx 8.2.3 is not backward compatible with sphinx 8.1.3 so we have to upgrade it as well.
This branch should be merged along with other branches updating sphinxcontrib packages. They are listed as dependency below.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.

⌛ Dependencies

#39686 this is a requirement for both sphinx 8.1.3 and 8.2.3
#39680 this is a sphinxcontrib package that was split from #39686 due to timing

@kiwifb
Copy link
Member Author

kiwifb commented Mar 18, 2025

The patch to sae_autodoc.py is big because the file has been completely reformatted with ruff upstream. I started with sphinx upstream and readded the sage bits as necessary.
I think I may now understand enough of the stuff to work on reducing sage_autodoc to essential class method override. But maybe I am full of myself there.

@dimpase
Copy link
Member

dimpase commented Mar 19, 2025

I am not able to reproduce your "builds dies horribly". With system Python 3.12.9, and all the sphinx-related Python packages from the more or less up to date Gentoo system, the only change I needed is already reported

--- a/src/sage_docbuild/ext/sage_autodoc.py
+++ b/src/sage_docbuild/ext/sage_autodoc.py
@@ -2976,7 +2976,7 @@ class PropertyDocumenter(DocstringStripSignatureMixin,  # type: ignore[misc]
 
 def autodoc_attrgetter(app: Sphinx, obj: Any, name: str, *defargs: Any) -> Any:
     """Alternative getattr() for types"""
-    for typ, func in app.registry.autodoc_attrgetters.items():
+    for typ, func in app.registry.autodoc_attrgettrs.items():
         if isinstance(obj, typ):
             return func(obj, name, *defargs)
 

What Python are you on? Could it be that something needs to be updated in sage the distro?

@kiwifb
Copy link
Member Author

kiwifb commented Mar 19, 2025

python 3.12.9. A couple of packages are missing to build the doc with 3.13.2, but sage builds and runs with 3.13.2 here otherwise. But it is possible there is a variation of something installed causing the issue. It would be nice to pin it down precisely but if my fix works for every case that's good enough for me. The only obvious package that comes to mind would be docutils which I have at 0.21.2.
Have you tried this branch rather than your fix?

@dimpase
Copy link
Member

dimpase commented Mar 19, 2025

let's compare outputs of sage --pip list
pl.txt

@kiwifb
Copy link
Member Author

kiwifb commented Mar 19, 2025

That's fun but I am not sure it is reliable considering it says you have sphinx 7.4.7.

@dimpase
Copy link
Member

dimpase commented Mar 20, 2025

oops ... it's correct. I forgot to clean it up.

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

LGTM. Please set it to positive review

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.

2 participants