Fix Sphinx cross-reference warnings#1177
Open
zain-asif-dev wants to merge 2 commits into
Open
Conversation
Enable sphinx.ext.intersphinx so references to standard library types resolve correctly, and correct several docstring cross-references that used bare class or method names where a fully qualified name was required to resolve in the context the docstring ends up being rendered in (including cases where a docstring is inherited by a subclass defined in a different module). Also add docstrings to a few DirectorySnapshot methods that were referenced but previously undocumented. The remaining handful of references that can never be resolved (a type hint that is only available unqualified due to postponed evaluation of annotations, an internal base class that is not part of the public API, and the platform specific observer implementations that can only be imported on their own platform) are listed in nitpick_ignore with an explanation. Signed-off-by: Zain Asif <zainasif.jutt1@gmail.com>
Collaborator
|
That's nice, thanks @zain-asif-dev. Could you also add a CI job to catch future regressions? |
Signed-off-by: Zain Asif <zainasif.jutt1@gmail.com>
Author
|
Good idea. The existing docs CI job already runs sphinx-build with -W (warnings as errors), but not in nitpicky mode (-n), and it turns out these particular reference warnings are only emitted in nitpicky mode. Without -n, sphinx-build silently renders unresolvable :class:/:meth: references as plain text instead of warning, so the previous CI wouldn't have caught this regression (or future ones). Added -n to the existing sphinx-build command in tox.ini so the current "Build the documentation" CI step now catches this. Verified it fails with 37 warnings on the code before this PR's fix, and passes cleanly with it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the
reference target not foundwarnings reported when building the documentation.sphinx.ext.intersphinxand mapped it to the Python standard library docs, so references to types likepathlib.Path,logging.Logger,re.Pattern, etc. resolve.FileSystemEventHandler.on_any_event(and similar methods) are inherited without their own docstring by classes inwatchdog.tricks, so a bare:class:FileSystemEvent`` in the base docstring fails to resolve once it is rendered under a different module.DirectorySnapshot.isdir,mtime, andsize, which were referenced from another docstring but were otherwise undocumented, and fixed:func:roles that should have been:meth:.Observer.scheduleinquickstart.rstto point at the method's actual defining class,BaseObserver.A few references can never be resolved and are listed in
nitpick_ignorewith an explanation:Path) because annotations are stored as strings due tofrom __future__ import annotations.watchdog.utils.bricks.SkipRepeatsQueue, an internal base class that isn't part of the public, documented API.inotify.InotifyObserver,fsevents.FSEventsObserver,kqueue.KqueueObserver,read_directory_changes.WindowsApiObserver), since only one of these can ever be importable on a given platform.Fixes #898
Testing
tox -e docs(runssphinx-build -aEWb html docs/source docs/build/html, which now succeeds with zero warnings)python -m sphinx -n -T -b man docs/source <outdir>(nitpicky mode) also succeeds with zero warningspytest(full suite): 170 passed, 10 skipped, 2 xpassed