Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sphinx warnings and turn warnings into errors #3290

Merged
merged 11 commits into from
Jan 26, 2021

Conversation

NicolasHug
Copy link
Member

Addresses part 1 of #3003 (comment)

This PR fixes sphinx warnings and sets the

napoleon_numpy_docstring = False
napoleon_google_docstring = True

for sphinx to issue warnings when the wrong parameter syntax is used.

The warnings are now turned into errors. Note: sphinx will now stop compilation at the first warning/error it sees; if we relied on sphinx>=8, we could use the --keep-going option to let it finish.

@@ -731,7 +731,7 @@ jobs:
conda activate ./env
pushd docs
pip install -r requirements.txt
make html
make html -SPHINXOPTS="-W"
Copy link
Member Author

@NicolasHug NicolasHug Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a better way would be to set

SPHINXOPTS = -W in doc/Makefile so that contributors would also see the errors when they run make html locally?

Or we can just tell them to compile the docs by running make html SPHINXOPTS="-W" in the relevant contributing guidelines

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on this @datumbox ? (It's marked as outdated but it's still relevant)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind either, it's your call.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, it's looking great. I only left one comment to get the input of the doc team.

docs/source/conf.py Show resolved Hide resolved
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR Nicolas.

@NicolasHug NicolasHug merged commit eb00e2a into pytorch:master Jan 26, 2021
@NicolasHug NicolasHug deleted the sphinx_warnings branch January 26, 2021 13:57
facebook-github-bot pushed a commit that referenced this pull request Feb 1, 2021
Summary: Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Reviewed By: datumbox

Differential Revision: D26156360

fbshipit-source-id: 001ecf0ae755140d8dca59f30e5940caf78484a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants