Repository navigation
Halve the time to build docs locally by inlining spell checking - #74549
Conversation
b6c14c7 to
733b47b
Compare
|
WHOA !! Fantastic !! Thanks @ashb !!!! |
|
And you can see the errors in CI !! . We should have thought about it way before !!! |
I honestly thought this would be way more difficult than it was. I had a lot of time waiting keeping an eye on the Task Loops stack (now thankfully landed!) and yeah, this turned out to be much easier. The one caveat to the error annotations: These do not ("cannot" right now) show up for generated API docs, as we don't have a way to to map api docs to source file it came from |
Andrushika
left a comment
Andrushika
left a comment
There was a problem hiding this comment.
WOW, thanks! Left one comment, there's one small detail that makes the new check give a different result from the old one.
733b47b to
d05147d
Compare
Andrushika
left a comment
Andrushika
left a comment
There was a problem hiding this comment.
Another super edge case found by my agent, but I would say it's non-blocking, since I couldn't find any existing example in our docs: an identifier followed by ... passes on main, but would fail after this PR.
example: isinstance...
main: isinstance... → isinstance → skipped (builtin)
this PR: isinstance… → isinstance… → reported
(SmartQuotes turns ... into …)
Previously to see the same error in docs build as you would get in CI, you would have to run both the docs build and the spellchecking build. The issue was that the spellcheck (using sphinx-contrib's spelling) was defined as a new builder, meaning you could either build HTML, or you can check spelling. On CI, this didn't result in any extra wall-clock time as the jobs were parallelized (but this change does remove a job from the overall matrix), but locally that was almost always not the case, so this would mean full build to view the HTML, and spell checking build to look for errors, resulting in it taking twice as long. The upstream spellchecker was also slower than it should be mainly because its importable-module filter calls find_spec on every distinct identifier-like word before consulting the dictionary. Here it runs only for words that are still misspelled. Word lists, filters and `:spelling:ignore:` behave as before. This also handles errors a lot more nicely (hence all the code deletions): - They are written natively to a JSON file, not scraped from a stream - They also show up as GH annotations (Where we can attribute it to a real file) to make fixing them easier on PRs. Please note: this is lightly breaking for Airflow contributors as `breeze build-docs --docs-only` and `--spellcheck-only` are gone -- every build checks spelling now as it is essentially free (on the order of milliseconds per module).
d05147d to
fe95212
Compare
|
Updated to unsmarten anything smartquotes did. |
|
Thanks :) |
|
#protm |
It only needs one nomination from anyone to get on the list, then the vote happens on the mailing list towards end of month, and yes, votes from everyone count there. |
4 participants
Previously to see the same error in docs build as you would get in CI,
you would have to run both the docs build and the spellchecking build.
The issue was that the spellcheck (using sphinx-contrib's spelling) was
defined as a new builder, meaning you could either build HTML, or you
can check spelling. On CI, this didn't result in any extra wall-clock
time as the jobs were parallelized (but this change does remove a job
from the overall matrix), but locally that was almost always not the
case, so this would mean full build to view the HTML, and spell checking
build to look for errors, resulting in it taking twice as long.
The upstream spellchecker was also slower than it should be mainly
because its importable-module filter calls find_spec on every distinct
identifier-like word before consulting the dictionary. Here it runs only
for words that are still misspelled. Word lists, filters and
:spelling:ignore:behave as before.This also handles errors a lot more nicely (hence all the code deletions):
real file) to make fixing them easier on PRs.
Please note: this is lightly breaking for Airflow contributors as
breeze build-docs --docs-onlyand--spellcheck-onlyare gone --every build checks spelling now as it is essentially free (on the order
of milliseconds per module).
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.