DOC: Speed up Sphinx build by using `sphinxcontrib-globalsubs` by namurphy · Pull Request #16162 · astropy/astropy

namurphy · 2024-03-05T22:34:39Z

Astropy presently uses docs/common_links.txt to store global substitutions that can be used throughout the documentation. The contents of this file are read in through docs/conf.py and then added to rst_prolog. Because rst_prolog gets added to the beginning of every reStructuredText file and docstring, a lengthy rst_prolog can add minutes to the documentation build (see #15149). 🐌

The sphinxcontrib-globalsubs extension allows us to create the global_substitutions dictionary in docs/conf.py to contain substitutions for use throughout the documentation, eliminating the need to prepend rst_prolog to files and docstrings.

A clean doc build prior to this PR locally took 7.2 min. After moving most of the common links to global_substitutions in docs/conf.py, a clean documentation build took 5.8 min. The doc build was shortened by 20%. 🎆 For these timings, I used make html -j 2 so that the documentation build was done on two processors. (These times do not include the creation of a Python environment.)

Because sphinxcontrib-globalsubs only works with substitutions, I had to convert some links in the docs to substitutions (i.e., SciPy_ → |SciPy|).

I still need to clean this up a bit & document this a bit better. There's still some content in docs/common_links.txt that will require a bit more work. I'd also like to define these substitutions in a separate file which can then get imported to docs/conf.py, but I had some trouble getting that to work, so I may do this in a follow-up PR.

Closes #15149. This was inspired by PlasmaPy/PlasmaPy#2277, PlasmaPy/PlasmaPy#2280, & PlasmaPy/PlasmaPy#2281. There's some related discussion on how to speed up the documentation build in CI in #16161.

github-actions · 2024-03-05T22:35:08Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

github-actions · 2024-03-05T22:35:10Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

pllim · 2024-03-05T22:54:21Z

For the record, the last latest build took 1038 seconds or 17.3 minutes (https://readthedocs.org/projects/astropy/builds/23646203/).

astrofrog · 2024-03-05T22:57:45Z

This is fantastic! I've always wondered how we could get rid of the rst_prolog so great to see this solution 😄

namurphy · 2024-03-05T23:35:11Z

This is fantastic! I've always wondered how we could get rid of the rst_prolog so great to see this solution 😄

Another win for procrastination-driven development! 😹

pllim · 2024-03-05T23:40:08Z

I just checked, the RTD build for this PR took 926 seconds or 15.4 mins. That's 10% speed up?

https://readthedocs.org/projects/astropy/builds/23658770/

namurphy · 2024-03-06T00:15:31Z

I just checked, the RTD build for this PR took 926 seconds or 15.4 mins. That's 10% speed up?

Thank you for checking this! The ∼20% improvement was on the sphinx-build step, and doesn't include the time needed to create the Python environment, etc.

It looks like the sphinx-build step itself was reduced from ∼710 s to ∼661 s on RTD. That's less of an improvement than I found locally, but I'll still take it!

eerovaher

I can approve for coordinates.

pllim · 2024-03-06T20:40:53Z

@namurphy , when do you think this is ready to be taken out of draft? Thanks!

namurphy · 2024-03-07T01:57:07Z

I'm still figuring out moving the remaining contents of common_links.txt over. The most recent RTD build gave the following errors several thousand times:

/home/docs/checkouts/readthedocs.org/user_builds/astropy/checkouts/16162/docs/<rst_epilog>:3: WARNING: py:obj reference target not found: global_substitutions
/home/docs/checkouts/readthedocs.org/user_builds/astropy/checkouts/16162/docs/<rst_epilog>:3: WARNING: py:obj reference target not found: docs/conf.py

I think...this can wait until tomorrow!

namurphy · 2024-03-07T16:19:39Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

namurphy · 2024-03-07T16:26:11Z

    "sphinx_design",
    "Jinja2>=3.1.3",
    "tomli; python_version < '3.11'",
+    "sphinxcontrib-globalsubs >= 0.1.1",


One possible concern is that this is an early version of sphinxcontrib-globalsubs, which was released last year. But, we've been using it in PlasmaPy for a while now with no problems, and it wouldn't be that much work to go back to common_links.txt.

Since this only affects doc build and RTD is green, probably okay.

namurphy · 2024-03-07T17:17:50Z

Ready for review! The most recent build took 878 s, which is about 15% less than the ∼1038 s baseline.

nstarman

Cosmo and Units LGTM. Thanks @namurphy for the significant speedup!

taldcroft

Fine for io.ascii and time.

pllim

Final timing:

This PR: Completed March 7, 2024. 4:20 p.m. Build took 878 seconds 🚀
main: Completed March 8, 2024. 2:46 a.m. Build took 1035 seconds

I think we have enough approvals. Let's speed things up. Thank you very much!

pllim · 2024-03-08T15:08:24Z

+    "StructuredUnit": ":class:`~astropy.units.StructuredUnit`",
+    # Utils
+    "Masked": ":class:`~astropy.utils.masked.Masked`",
+    # Minimum versions


I think we lost the ability to be able to dynamically insert new minversion listing for dependencies (which tend to keep growing nowdays).

That said, docs/conf.py is easier to discover than docs/common_links.txt in general, plus the speed up. So the benefit of this PR outweighs the risk of forgetting to insert a new entry here.

pllim · 2024-03-08T15:09:48Z

-<https://www.python.org/dev/peps/pep-0008/>`_. All of the PEP8 testing
+so that they check Python source code for |PEP8|. All of the PEP8 testing
 options require the `pytest-pep8 plugin
 <https://pypi.org/project/pytest-pep8>`_, which must be installed


Side note: I never even installed pytest-pep8. The dev guide could probably use some serious clean-up but that is a battle for another day.

pllim · 2024-03-08T15:11:25Z

    "sphinx_design",
    "Jinja2>=3.1.3",
    "tomli; python_version < '3.11'",
+    "sphinxcontrib-globalsubs >= 0.1.1",


Since this only affects doc build and RTD is green, probably okay.

* Apply sphinxcontrib-globalsubs to substitutions across the package * Remove unused substitutions/links * Add comments to conf.py * Move over common_links.rst to conf.py

namurphy added 4 commits March 5, 2024 15:48

Apply sphinxcontrib-globalsubs to substitutions

a21e2ce

Fix problems in keys for global_substitutions

5f91f24

Apply global substitutions across the package

f155878

Change one more substitution

091d705

github-actions Bot added testing Docs io.fits time io.ascii units coordinates cosmology installation samp visualization.wcsaxes labels Mar 5, 2024

namurphy added 2 commits March 5, 2024 17:38

Run pre-commit

2657ed3

Remove unused substitutions/links

e5ec63f

pllim added this to the v6.1.0 milestone Mar 5, 2024

pllim added no-changelog-entry-needed Performance labels Mar 5, 2024

namurphy changed the title ~~DOC: Speed up documentation build by ∼20% by using sphinxcontrib-globalsubs~~ DOC: Speed up Sphinx build by ∼20% by using sphinxcontrib-globalsubs Mar 5, 2024

namurphy changed the title ~~DOC: Speed up Sphinx build by ∼20% by using sphinxcontrib-globalsubs~~ DOC: Speed up Sphinx build by using sphinxcontrib-globalsubs Mar 6, 2024

eerovaher approved these changes Mar 6, 2024

View reviewed changes

namurphy added 2 commits March 6, 2024 20:33

Add comment to common_links.txt

7130f77

Add comments to conf.py

5c19f18

Move over common_links.rst to conf.py

62e59b1

[pre-commit.ci] auto fixes from pre-commit.com hooks

30cd926

for more information, see https://pre-commit.ci

namurphy commented Mar 7, 2024

View reviewed changes

namurphy mentioned this pull request Mar 7, 2024

DOC: Add a CI check to rebuild documentation from a cached documentation build #16161

Closed

namurphy marked this pull request as ready for review March 7, 2024 17:17

namurphy requested review from a team, Cadair, astrofrog, dhomeier, larrybradley, saimn and taldcroft as code owners March 7, 2024 17:17

namurphy requested a review from nstarman March 7, 2024 17:17

nstarman approved these changes Mar 7, 2024

View reviewed changes

taldcroft approved these changes Mar 8, 2024

View reviewed changes

pllim approved these changes Mar 8, 2024

View reviewed changes

pllim merged commit a173fbf into astropy:main Mar 8, 2024

namurphy deleted the sphinx-globalsubs branch March 14, 2024 21:22

pllim mentioned this pull request Aug 25, 2025

DOC: Use numpydoc, not napoleon for API docstring rendering spacetelescope/jwst#9767

Merged

2 of 10 tasks

Conversation

namurphy commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 5, 2024

Uh oh!

github-actions Bot commented Mar 5, 2024

Uh oh!

pllim commented Mar 5, 2024

Uh oh!

astrofrog commented Mar 5, 2024

Uh oh!

namurphy commented Mar 5, 2024

Uh oh!

pllim commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

namurphy commented Mar 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eerovaher left a comment

Choose a reason for hiding this comment

Uh oh!

pllim commented Mar 6, 2024

Uh oh!

namurphy commented Mar 7, 2024

Uh oh!

namurphy commented Mar 7, 2024

Uh oh!

namurphy Mar 7, 2024

Choose a reason for hiding this comment

Uh oh!

pllim Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

namurphy commented Mar 7, 2024

Uh oh!

nstarman left a comment

Choose a reason for hiding this comment

Uh oh!

taldcroft left a comment

Choose a reason for hiding this comment

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

pllim Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

pllim Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

pllim Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

namurphy commented Mar 5, 2024 •

edited

Loading

pllim commented Mar 5, 2024 •

edited

Loading

namurphy commented Mar 6, 2024 •

edited

Loading