Fix doc build problems (part 1)#3811
Conversation
This resolves a local docs build error: ```console $ python -m venv venv-docs $ source venv-docs/bin/activate $ python -m pip install --group=docs $ make docs ... no theme named 'sphinx_rtd_theme' found (missing theme.toml?) ... $ deactivate ```
This resolves an info message when building the docs: ``` intersphinx inventory has moved: https://pytorch.org/docs/stable/objects.inv -> https://docs.pytorch.org/docs/stable/objects.inv ```
The warnings are: ``` WARNING: missing attribute modality_config in object sentence_transformers.base.modules.Transformer [autodoc] WARNING: missing attribute transformer_task in object sentence_transformers.base.modules.Transformer [autodoc] ``` While these attributes exist on instances, they are not methods nor properties.
The warning is: ``` sentence_transformers/sentence_transformer/losses/multiple_negatives_ranking.py: docstring of sentence_transformers.sentence_transformer.losses.multiple_negatives_ranking.MultipleNegativesRankingLoss:52: ERROR: Unexpected indentation. [docutils] ``` RST requires nested lists to be separated by blank lines.
The warning is: ``` sentence_transformers/sentence_transformer/losses/cached_multiple_negatives_ranking.py: docstring of sentence_transformers.sentence_transformer.losses.cached_multiple_negatives_ranking.CachedMultipleNegativesRankingLoss:46: ERROR: Unexpected indentation. [docutils] ``` RST requires nested lists to be separated by blank lines.
The warning is: ``` docs/package_reference/sparse_encoder/index.rst:4: WARNING: toctree contains reference to nonexisting document 'docs/package_reference/sentence_transformer/sampler' [toc.not_readable] ``` The `sampler.md` file was moved in commit `4cdbd836717e420e3b4c2ea72c894fb1e2120100`.
The warning is: ``` sentence_transformers/sparse_encoder/model.py: docstring of sentence_transformers.sparse_encoder.model.SparseEncoder.decode:12: ERROR: Unexpected indentation. [docutils] ``` The root issue was the formatting of the conditional return types. Sphinx could not handle two type annotations on separate lines, and ultimately rendered the content incorrectly.
The warnings are: ``` index.rst:7: WARNING: Duplicate explicit target name: "quickstart". [docutils] index.rst:7: WARNING: Duplicate explicit target name: "quickstart". [docutils] index.rst:17: WARNING: Duplicate explicit target name: "quickstart". [docutils] ``` The issue is that links created with a single trailing underscore are registered with Sphinx. The solution is to use two trailing underscores which create anonymous links. This avoids the "duplicate explicit target name" warning.
613877d to
8766f06
Compare
|
Rebased on |
|
Hello! Thank you for this, I see a lot of small oversights that I've made in various refactors over the past year or so. I'll have to review this in more detail tomorrow (I think tomorrow, don't quote me on that 😄) and in the meanwhile, Copilot can have a look.
|
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple Sphinx documentation build failures/warnings so the docs can successfully progress through the “reading sources” stage, primarily by fixing missing dependencies and resolving Sphinx/autodoc/reST formatting issues.
Changes:
- Adds
sphinx_rtd_themeto the docs dependency group to fix missing theme build failures. - Fixes/adjusts Sphinx inputs (reST links, toctrees, intersphinx mapping) to remove common build warnings/errors.
- Updates autodoc/member listings and docstrings to avoid missing-attribute and indentation-related doc build issues.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
sentence_transformers/sparse_encoder/model.py |
Refines decode() docstring return type formatting to be Sphinx-friendly and clearer. |
sentence_transformers/sentence_transformer/losses/multiple_negatives_ranking.py |
Adjusts docstring spacing/formatting to prevent Sphinx indentation/parsing issues. |
sentence_transformers/sentence_transformer/losses/cached_multiple_negatives_ranking.py |
Same docstring formatting adjustments as the non-cached loss variant. |
pyproject.toml |
Adds sphinx_rtd_theme to the docs dependency group to unblock doc builds. |
index.rst |
Fixes duplicate explicit target name issues by using anonymous links where needed. |
docs/package_reference/sparse_encoder/index.rst |
Updates toctree entry to point at the correct sampler document location. |
docs/package_reference/base/modules.rst |
Removes autodoc :members: entries that are not class attributes (avoids missing-attribute errors). |
docs/conf.py |
Updates the PyTorch intersphinx base URL to match the current inventory location. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tomaarsen
left a comment
There was a problem hiding this comment.
Clear improvements here! I've proposed some minor changes that I'll just merge straight away, they're simple enough.
Co-authored-by: Tom Aarsen <37621491+tomaarsen@users.noreply.github.com>
This PR resolves a number of documentation build problems:
no theme named 'sphinx_rtd_theme' foundintersphinx inventory has movedmissing attribute <NAME> in objectUnexpected indentationtoctree contains reference to nonexisting documentDuplicate explicit target nameThis fixes all doc build warnings and errors up to, and including, the "reading sources" stage of the Sphinx builds. These steps can be used to confirm the changes before and after:
It is recommended that this PR be reviewed commit-by-commit, which will highlight the specific issue being resolved (each warning/error is included in the commit body, along with an explanation), together with its targeted fix.
There are still a significant number of issues to address, which appear in the "checking consistency" and "writing output" stages. My goal is to resolve all of the docs warnings, and then add an additional CI workflow to test doc builds, with warnings escalated to errors. This will help prevent new formatting/rendering/missing document problems (see for example commit 4cdbd83, which renamed
sampler.mdbut overlooked a table of contents entry update).BTW, thanks for reviewing these PRs! I'm enjoying the quick iteration time!