plotly / plotly.py

The interactive graphing library for Python :sparkles: This project now includes Plotly Express!
https://plotly.com/python/
MIT License
16.26k stars 2.55k forks source link

enable support for more recent versions of sphinx for documentation generation #4085

Open kloczek opened 1 year ago

kloczek commented 1 year ago

Looks like plotly documentatio ncannot be generated using latest sphinx 61.3.

+ /usr/bin/sphinx-build -n -T -b man doc/apidoc build/sphinx/man
Running Sphinx v6.1.3
WARNING: Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).
making output directory... done

Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/sphinx/cmd/build.py", line 279, in build_main
    app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
  File "/usr/lib/python3.8/site-packages/sphinx/application.py", line 237, in __init__
    self.config.setup(self)
  File "/home/tkloczko/rpmbuild/BUILD/plotly.py-5.13.1/doc/apidoc/conf.py", line 209, in setup
    app.add_stylesheet("plotly-style.css")  # also can be a full URL
AttributeError: 'Sphinx' object has no attribute 'add_stylesheet'

Exception occurred:
  File "/home/tkloczko/rpmbuild/BUILD/plotly.py-5.13.1/doc/apidoc/conf.py", line 209, in setup
    app.add_stylesheet("plotly-style.css")  # also can be a full URL
AttributeError: 'Sphinx' object has no attribute 'add_stylesheet'
The full traceback has been saved in /tmp/sphinx-err-cu42fgnj.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
nicolaskruchten commented 1 year ago

Yes, we pin Sphinx to a lower version to generate https://plotly.com/python-api-reference/index.html

kloczek commented 1 year ago

Sphinx 6.0.0 has been released exactly 4 months ago so .. 🤔 sonner or later it will be nescessary to move to new sphinx version. 😋 Do you have any plans to do that soon? (just asking)

nicolaskruchten commented 1 year ago

4 months is basically no time at all! It's unreasonable to expect us to track all the different major-version bumps of other packages in the ecosystem, especially within just a few months. We have no plans to move to a new version of Sphinx, no (and I don't think it will ever be "necessary")... we use it to generate the site mentioned above and make no promises about guaranteeing that other people can use it to do other things :)

dh3968mlq commented 6 months ago

I've been thinking about trying to do this update. The Sphinx version currently used (3.5.4) limits the Python version to be < 3.10 (https://github.com/sphinx-doc/sphinx/issues/9562)

I'm wondering might this be made easier it there could be two separate virtualenvs, one for the apidoc build and one for the tutorials build?

(At the moment there is a single virtualenv (doc/requirements.txt) used for both. The api_doc build needs Sphinx and very little else, and the tutorials doc build uses a lot of other packages but not, I believe, Sphinx)

It would then be possible to update the Sphinx version for the API doc and move to a current version of Python without having to worry about whether this has broken the build of the tutorials doc. And it might also make it easier for someone to update the tutorials doc to a newer version of Python if the Sphinx dependency can be removed.

I think the switch to two separate environments would need a new requirements.txt in doc/apidoc (initially a copy of doc/requirements.txt), changes to .circlcei/config.yml so that apidoc gets built using the new requirements.txt, and changes to doc/README.md

But I don't feel as a (new) community contributor I should be making changes to .circleci/config.yml

Would there be any support for making this change? Apologies if I've missed something that makes this either misguided or more complex than I've realised.

Coding-with-Adam commented 6 months ago

hi @dh3968mlq Is the Sphinx version a blocker for you?

dh3968mlq commented 6 months ago

Is the Sphinx version a blocker for you?

On reflection, no it isn't at the moment, and I may have been focussing on the wrong thing in my previous comment.

I can look at upgrading the apidoc to a more recent Sphinx version (if I find enough free time) without any immediate changes to the CI, and if I produce anything worthwhile I'll come back here to discuss further.

The error in the original post in this issue needs a one-line change in conf.py (add_stylesheet() has been replaced by add_css_file()), and the non-functioning dropdowns in the Bootstrap theme example at https://sphinx-themes.org/sample-sites/sphinx-bootstrap-theme/# are also a one-line fix (add extension sphinxcontrib.jquery). But a few other things go wrong building the apidoc on Sphinx 7 that need a bit more time and effort.

dh3968mlq commented 5 months ago

The bootstrap Sphinx theme currently used in the apidocs hasn't been updated for a while, and it looks like it's not going to work properly on new versions of Sphinx.

I've done some work using the RTD theme instead and it seems this can be made to look similar to the other Plotly Docs (e.g. https://plotly.com/python/plotly-express/) without many code changes, using Sphinx 7 and Python 3.11. See image below, which shows the updated version of https://plotly.com/python-api-reference/

Adam has suggested it may be worth working this up into a pull request (it still needs a little work), and I'd appreciate any further feedback on whether there's a future in this route. I can also put a complete build of the apidoc using this theme onto Github if there's interest in it.

apidoc-rtd-2024-05-14

kloczek commented 5 months ago

As I;m interested of only roff output (man page) .. is it possible to make minor release with that change? 🤔

dh3968mlq commented 5 months ago

@kloczek I can't see any reference to roff in this repo - is this something you build yourself or gets built elsewhere in the plotly/dash repos?

The changes I'm suggesting here don't change the content of the apidoc at all - they only update the look and feel of the html

kloczek commented 5 months ago

Roff output (AKA man page) is one of the standard output formats supported by sphinx.

I've mentioned about that because fixing issue and uploading new html content to module doc page is not enough and it would be good to have new release with that fix 😋

dh3968mlq commented 5 months ago

If you want to build the man page (or, for that matter, the html) yourself you can do that by cloning the plotly.py fork at https://github.com/dh3968mlq/plotly.py-apidoc, and use branch dev-apidoc-dh3968

There's guidance on environment setup in doc/apidoc/envsetup and doc/apidoc/requirements.txt

If you do this I'd appreciate feedback - the man page appears to build and look OK to me, but I'm not familiar enough with it to expect to spot all issues.

In the longer term, I don't know as yet if this approach will be adopted into releases at all.

kloczek commented 5 months ago

Is it possible to merge those changes here? 🤔

Coding-with-Adam commented 5 months ago

Hi @dh3968mlq Hi @kloczek We'll be talking about this issue next week and get back to you.

dh3968mlq commented 5 months ago

Hi @Coding-with-Adam , thanks for the response

I realise I need advice anyway on creating a pull request for these changes - they will inevitably not pass tests as they stand, since they need a modified environment for the apidoc build. So I'll wait for you to have that discussion before creating a pull request.

In the meantime I've put a copy of the new build apidoc online at https://plotly-apidoc-alpha-sphinx7-e1bad215977b.herokuapp.com/. Hopefully this will be useful in your discussion. (Uses eco dynos, may take a few seconds to wake up)

It's a step forward, I feel, and is easier to navigate and looks closer to other Plotly docs in comparison to the currently released apidoc. But it clearly doesn't solve all the issues with the apidoc, and in particular it seems to me that tables of contents, navigation and typography need more attention. I think it's quite likely that moving to Sphinx 7 might open the door to making those improvements, but I can also see why you might want to go a different route.