-
Notifications
You must be signed in to change notification settings - Fork 504
Switch to pydata-sphinx-theme #3165
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
base: main
Are you sure you want to change the base?
Conversation
adamjstewart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks decent, fixes many of our bugs. Quite different from our previous theme, so may take some getting used to. Hasn't had a new release in almost a year, so either very stable or unmaintained.
I think there are a few other things we'll like to customize, such as default pages for different tabs and the version selection menu, but we can do those if we decide this is the style we want.
If it's not too much work, it would be nice to see a few other alternatives that are either simple or widely used by the community.
| 'navigation_depth': 4, | ||
| 'navbar_align': 'left', | ||
| 'header_links_before_dropdown': 6, | ||
| 'icon_links': [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are cool!
addb7b1 to
415f278
Compare
|
One note on the landing page (index). Currently it shows the docstring in torchgeo/__init__.py (left image) but I could show the README instead (right image) |
adamjstewart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaning towards this theme because it matches the rest of the PyTorch ecosystem but is still maintained. A couple remaining questions:
- Are we able to remove any of the files in
docs/_static? These were CSS hacks designed to fix bugs in pytorch-sphinx-theme. - I'm curious if there's a way to get rid of the version switcher in the bottom right corner and add one to the top menu bar like pytorch2-sphinx-theme. It's possible this is also a cache issue on my side.
| # Sphinx 5.3+ required to allow section titles inside autodoc class docstrings | ||
| # https://github.com/sphinx-doc/sphinx/pull/10887 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also be okay with removing section headings from docstrings, I don't imagine it happens in very many places.
.readthedocs.yaml
Outdated
| tools: | ||
| # Sphinx 6.1 and older do not support Python 3.13+ | ||
| python: '3.12' | ||
| python: '3.13' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can test 3.14 now if you rebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs #3203 to be merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically that shouldn't matter, the pytest warning filters don't apply to sphinx anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried switching to 3.14 earlier and the lightly warnings caused the RtD build to fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that might be a problem. I don't know how to tell RtD to ignore that, #3203 shouldn't help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were right. Warnings are still raised. I'll proceed with building docs on 3.13 for now
89004f4 to
35b6ea0
Compare
8bbde1c to
0b8eb87
Compare
This reverts commit 0b8eb87.
|
Note: Currently colors are based on the default pydata-sphinx-theme i.e. blue and purple. This can be overridden with a custom css. Can be done in a later PR |
|
Python 3.14 error is: This looks like sphinx-doc/sphinx#13701. So Sphinx 9+ is required for Python 3.14+. |
Only thing stopping us from supporting sphinx 9+ is |
|
Drop the README then 🙂 |
|
Docs now work with Python 3.14. There is an issue with tutorials test though. |
Fixes #2006
Widely used across the scientific Python ecosystem. Major libraries using this theme include NumPy, Pandas, SciPy, GeoPandas, Xarray etc.
pytorch_sphinx_theme2inherits from this theme (link) as well.