Switch to using Tox #1459
Switch to using Tox #1459
Conversation
| os.environ['QISKIT_DOCS'] = 'TRUE' | ||
| if sys.platform == 'darwin': | ||
| os.environ['QISKIT_IN_PARALLEL'] = 'TRUE' |
There was a problem hiding this comment.
These env vars aren't used by anything. Bad copy and paste.
| cell_timeout = int(os.getenv('QISKIT_CELL_TIMEOUT', 180)) | ||
| nbsphinx_timeout = cell_timeout | ||
| nbsphinx_timeout = 300 | ||
| nbsphinx_execute = 'always' | ||
| nbsphinx_execute_arguments = [ | ||
| "--InlineBackend.figure_formats={'png', 'pdf'}", | ||
| "--InlineBackend.rc={'figure.dpi': 96}", | ||
| ] | ||
|
|
||
| nbsphinx_thumbnails = { | ||
| } | ||
|
|
||
| nbsphinx_widgets_path = "" | ||
| html_sourcelink_suffix = "" | ||
| nbsphinx_thumbnails = {"**": "_static/no_image.png"} |
There was a problem hiding this comment.
I aligned this config with qiskit-metapackage, since that is what actually ends up being used in the final rendered docs.
There was a problem hiding this comment.
Great! This solve the thumbnail issue in the local build. In general, it's nice to see the same output in local build of the tutorials repo alone as the final rendered docs built by the metapackage.
| # | ||
| # This is also used if you do content translation via gettext catalogs. | ||
| # Usually you set "language" from the command line for these cases. | ||
| language = None |
There was a problem hiding this comment.
Bad config in Sphinx 6+
There was a problem hiding this comment.
Nice. CI is using Sphinx 7.0 instead of 5.3
| envlist = py310,py39,py38 | ||
|
|
||
| [testenv] | ||
| no_package = true |
There was a problem hiding this comment.
This is because we don't distribute any Python package in this repo. It's solely docs.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This reverts commit 2d1ff82.
Eric-Arellano
changed the title
Eric-Arellano
requested review from
HuangJunye,
mtreinish and
1ucian0
and removed request for
HuangJunye
There was a problem hiding this comment.
Thanks for working on this. It's great to reduce the duplication of dependencies in azure-pipelines.yml and making the local build closer to the final build by the metapackage.
Can you also update the instructions about how to build the documentation in readme to reflect the change? After that it should be ready for merging.
| cell_timeout = int(os.getenv('QISKIT_CELL_TIMEOUT', 180)) | ||
| nbsphinx_timeout = cell_timeout | ||
| nbsphinx_timeout = 300 | ||
| nbsphinx_execute = 'always' | ||
| nbsphinx_execute_arguments = [ | ||
| "--InlineBackend.figure_formats={'png', 'pdf'}", | ||
| "--InlineBackend.rc={'figure.dpi': 96}", | ||
| ] | ||
|
|
||
| nbsphinx_thumbnails = { | ||
| } | ||
|
|
||
| nbsphinx_widgets_path = "" | ||
| html_sourcelink_suffix = "" | ||
| nbsphinx_thumbnails = {"**": "_static/no_image.png"} |
There was a problem hiding this comment.
Great! This solve the thumbnail issue in the local build. In general, it's nice to see the same output in local build of the tutorials repo alone as the final rendered docs built by the metapackage.
| # | ||
| # This is also used if you do content translation via gettext catalogs. | ||
| # Usually you set "language" from the command line for these cases. | ||
| language = None |
There was a problem hiding this comment.
Nice. CI is using Sphinx 7.0 instead of 5.3
|
|
||
| [testenv:docs] | ||
| commands = | ||
| sphinx-build -j auto -b html {toxinidir} {toxinidir}/_build/html/ |
There was a problem hiding this comment.
Should we use -W flag as well or this repo is not ready for that yet?
There was a problem hiding this comment.
This is the goal. There is one remaining issue that I plan to fix in a follow up PR -- still a WIP.
|
You mentioned that nbsphinx is not working on your mac. Does it mean that you can't make a successful local build using tox on your mac? |
Exactly. But I time-boxed trying to figure this out—there are a few other higher priorities I'm focused on. The main reason I'm cleaning up this repo is to unblock enabling Some forward progress is better than none. While I couldn't get local to work on my mac, CI at least still works. |
Summary
Previously, there was no standardized way to build the docs locally. While Tox isn't perfect, it aligns us with the rest of the Qiskit ecosystem.
Switching to Tox is important here because it de-duplicates our requirements, which were specified both in requirements-dev.txt and again in azure-pipelines.yaml.
Details and comments
This also fixes two Sphinx warnings due to bad config in
conf.py.I couldn't actually get NBSphinx to work on my mac due to import errors. But this is still some forward progress, so I put up the PR. CI at least will now use tox, and continue to produce a zip of the docs that CI users can inspect.