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

DOC: Some features of docs built with sphinx 6 do not work #23647

Closed
F3eQnxN3RriK opened this issue Apr 23, 2023 · 18 comments · Fixed by #23677
Closed

DOC: Some features of docs built with sphinx 6 do not work #23647

F3eQnxN3RriK opened this issue Apr 23, 2023 · 18 comments · Fixed by #23677

Comments

@F3eQnxN3RriK
Copy link
Contributor

Issue with current documentation:

The pulldown and search box do not work in documentation built with sphinx 6.

Idea or request for content:

These work fine with sphinx 5, so there seems to be a breaking change in sphinx 6 that causes the problem. The sphinx version is currently fixed at 5 (gh-23640), but it would be nice to be able to use sphinx 6 for the build.

@mattip
Copy link
Member

mattip commented Apr 23, 2023

I see the pydata-sphinx-theme tests are passing with 6.1.3, I wonder what is going wrong. Who would be a likely theme expert to get some eyes on this? @drammock?

@drammock
Copy link
Contributor

Regarding the "pulldown" I guess you mean the version switcher dropdown? AFAIK that never works on local builds for any project, because of CORS restrictions.

Here's a screenshot showing that search works for me on a local build:

Screenshot 2023-04-23 at 12-00-43 Search — NumPy v1 25 dev0 Manual

(further down in the search results, the preview text for pages like "global state" or "indexing on ndarrays" does not display, I believe also due to CORS restrictions).

$ mamba list sphinx
# packages in environment at /opt/miniforge3/envs/npdev:
#
# Name                    Version                   Build  Channel
pydata-sphinx-theme       0.9.0              pyhd8ed1ab_1    conda-forge
sphinx                    6.1.3              pyhd8ed1ab_0    conda-forge
sphinx-design             0.4.1              pyhd8ed1ab_0    conda-forge
[rest omitted...]

note that I had to do some manual stuff to get this env to even install, as both breathe and sphinx-design still try to force sphinx<6 when installing with mamba (for sphinx-design at least I know the installed version is supposed to be sphinx-6 compatible... not sure what's up with that).

I've tried a few times to wrap my head around CORS and how to allow things like loading the version dropdown JSON on local builds (or even on PR builds, which often doesn't work either). I've always failed; if anyone reading this understands how CORS works and what would be necessary please speak up.

@mattip
Copy link
Member

mattip commented Apr 25, 2023

Since we pinned the devdocs to sphinx 5 in #23640, it is hard to debug a broken deployment. I submitted #23658 to revert the max pinning

@mattip
Copy link
Member

mattip commented Apr 25, 2023

@drammock you can see the devdocs are currently broken after updating to sphinx 6. When I look in the devtools javascript console, I see some javascript errors. The first is in the search function:

Uncaught ReferenceError: $ is not defined
    <anonymous> https://numpy.org/devdocs/:171
    <anonymous> https://numpy.org/devdocs/:209
[devdocs:171:5](https://numpy.org/devdocs/)

and then there is this too

Uncaught ReferenceError: jQuery is not defined
    <anonymous> https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:1
    n https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:1
    <anonymous> https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:32
    n https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:1
    <anonymous> https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:26
    n https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:1
    <anonymous> https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:1
    <anonymous> https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695:1
[pydata-sphinx-theme.js:1:927](https://numpy.org/devdocs/_static/scripts/pydata-sphinx-theme.js?digest=92025949c220c2e29695)

​

@anshuman0123
Copy link

Hey , I am new to open source . Please guide me how can I contribute in this repo

@mattip
Copy link
Member

mattip commented Apr 25, 2023

Hi @anshuman. We have some resources on our community page, and ways to contribute on the contribution page. You might want to join our next newcomer's meeting MAy 4, which will be announced on this mail archive thread a few days before.

@drammock
Copy link
Contributor

Sphinx 6 no longer automatically includes JQuery (changelog, first item under "incompatible changes"). pydata-sphinx-theme version 0.9.0 assumes that it is present, so you'll need to add JQuery. Ways to do this:

  • app.add_js_file('path/to/file.js') in the setupfunction at the end ofconf.py`. Might require hosting/vendoring a copy of jquery
  • there is a sphinxcontrib-jquery extension that will inject it for you (I've not used this)
  • add a new Jinja template to numpy: doc/source/_templates/layout.html that inherits from the theme template but adds an appropriate <script> or <link> tag to load JQuery.

I can make the PR to numpy tomorrow if you like, just LMK which option you prefer. If unsure, I'd probably pick the 3rd option myself (because I'm familiar with how the templates work)

@mattip
Copy link
Member

mattip commented Apr 27, 2023

Is there a reason we are still with the older version of pydata-sphinx-theme? Maybe we should update to v0.13.3?

@rossbar
Copy link
Contributor

rossbar commented Apr 27, 2023

Is there a reason we are still with the older version of pydata-sphinx-theme? Maybe we should update to v0.13.3?

Not that I'm aware of. Presumably the most recent version of the theme includes the necessary JQuery to make these features work? It's worth a shot at least - hopefully this is just fixed by updating. If not, then perhaps we should raise the issue there if it hasn't been already?

@drammock
Copy link
Contributor

Presumably the most recent version of the theme includes the necessary JQuery

Actually newer versions of the theme remove our use of jQuery (replaced with vanilla JavaScript). But it amounts to the same thing (Sphinx 6 compatibility)

Is there a reason we are still with the older version of pydata-sphinx-theme?

Scipy has had issues with increased build time on later versions of the theme. These have been difficult to reproduce and hence difficult to fix. I'll do some local tests to see if numpy is likely to suffer the same problem. If not, updating the theme pin seems like the best option.

Crossrefs:

@tupui
Copy link
Contributor

tupui commented Apr 27, 2023

@drammock thanks for having a look 🙇

I would expect the same issues to arise as we try to keep both SciPy and NumPy's docs in sync. That will definitely be interesting if your tests show now slowdown here.

@anshuman
Copy link

@anshuman0123 I guess this @mattip's comment was meant for you:

Hi @anshuman. We have some resources on our community page, and ways to contribute on the contribution page. You might want to join our next newcomer's meeting MAy 4, which will be announced on this mail archive thread a few days before.

@drammock
Copy link
Contributor

building numpy docs with sphinx 6 and pydata-sphinx-theme 0.13.3 I get

$ time make html
[...]
real	3m8.449s
user	3m11.327s
sys	0m33.501s

The good news is that after this, make show gives a local build with a working version dropdown and no browser console errors on the homepage... there are still JS errors on the search results page, but they are all like this:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at file:///opt/numpy/doc/build/html/user/c-info.python-as-glue.html. (Reason: CORS request not http).

followed by a related/resultant error

Uncaught (in promise) TypeError: NetworkError when attempting to fetch resource.

These search page errors all originate from either _static/searchtools.js or _static/doctools.js. I have not looked at the contents of those files yet, and for now I assume that the errors will go away if the website is being properly served.

To me this suggests that updating the theme pin to 0.13.3 will work just fine for NumPy (with the caveat that some colors have changed in the interim so you'll likely want to do a few CSS tweaks).

The times compare favorably (after a make clean) against building with sphinx 6 and pydata-sphinx-theme 0.9.0:

$ time make html
[...]
real	3m20.908s
user	3m24.088s
sys	0m33.130s

@mattip
Copy link
Member

mattip commented Apr 27, 2023

Cool - thanks for checking. Since you already have working code, would you be able to create a PR?

@drammock
Copy link
Contributor

yep, working on it now. Since I'm not staring at numpy docs every day I'll need to rely on reviewers to make sure I catch all the needed CSS changes. Is there a short list of pages (or a "kitchen sink" page) that I should pay special attention to when comparing against current stable docs?

@rossbar
Copy link
Contributor

rossbar commented Apr 27, 2023

yep, working on it now. Since I'm not staring at numpy docs every day I'll need to rely on reviewers to make sure I catch all the needed CSS changes. Is there a short list of pages (or a "kitchen sink" page) that I should pay special attention to when comparing against current stable docs?

IMO I wouldn't worry about style tweaks too much... I would generally prefer to go with theme defaults as much as possible to avoid maintenance burden!

@tupui
Copy link
Contributor

tupui commented Apr 27, 2023

Oh great new for NumPy if there is no build time difference!

@mattip
Copy link
Member

mattip commented Apr 27, 2023

Closing, PR #23677 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants