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

Hundreds of Sphinx errors #3370

Closed
crusaderky opened this issue Oct 3, 2019 · 14 comments · Fixed by #3602
Closed

Hundreds of Sphinx errors #3370

crusaderky opened this issue Oct 3, 2019 · 14 comments · Fixed by #3602
Labels
CI Continuous Integration tools stale topic-documentation

Comments

@crusaderky
Copy link
Contributor

crusaderky commented Oct 3, 2019

sphinx-build emits a ton of errors that need to be polished out:

https://readthedocs.org/projects/xray/builds/ -> latest -> open last step

Options for the long term:

  • Change the "Docs" azure pipelines job to crash if there are new failures. From past experience though, this should come together with a sensible way to whitelist errors that can't be fixed. This will severely slow down development as PRs will systematically fail on such a check.
  • Add a task in the release process where, immediately before closing a release, the maintainer needs to manually go through the sphinx-build log and fix any new issues. This would be a major extra piece of work for the maintainer.

I am honestly not excited by either of the above. Alternative suggestions are welcome.

@crusaderky crusaderky changed the title Hundreds os Sphinx errors Hundreds of Sphinx errors Oct 3, 2019
@crusaderky crusaderky added CI Continuous Integration tools topic-documentation labels Oct 3, 2019
@dcherian
Copy link
Contributor

Is there a sphinx or restructured text linter that we can run? e.g. https://github.com/PyCQA/doc8

@max-sixty
Copy link
Collaborator

Do these matter?

Do we know of examples of bad outcomes (e.g. hard-to-read docs)?

@crusaderky
Copy link
Contributor Author

Yes. For example

http://xarray.pydata.org/en/stable/generated/xarray.Dataset.integrate.html#xarray.Dataset.integrate

The sphinx log states:

/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/latest/lib/python3.7/site-packages/xarray-0.14.0+37.g96cc2bc6-py3.7.egg/xarray/core/dataset.py:docstring of xarray.Dataset.integrate:12: WARNING: Unexpected indentation.

What I just now noticed is that several docstrings may be imported from numpy - which complicates everything.

@max-sixty
Copy link
Collaborator

The sphinx log states:

/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/latest/lib/python3.7/site-packages/xarray-0.14.0+37.g96cc2bc6-py3.7.egg/xarray/core/dataset.py:docstring of xarray.Dataset.integrate:12: WARNING: Unexpected indentation.

That's a good case...

@max-sixty
Copy link
Collaborator

I ran https://github.com/myint/docformatter on our code docformatter --in-place -r --wrap-summaries=88 . ; results here

But it doesn't solve many of our issues, which are around the sphinx docstrings

Happy to do a PR, but worry it's more churn than help?

@max-sixty
Copy link
Collaborator

Is there a sphinx or restructured text linter that we can run? e.g. https://github.com/PyCQA/doc8

Looks reasonable; though this is a checker rather than fixer

@keewis keewis mentioned this issue Nov 13, 2019
3 tasks
@keewis
Copy link
Collaborator

keewis commented Dec 6, 2019

most of the nitpicky errors are raised because numpydoc tries to link to parameter types. We can define aliases and ignore terms like instance or words like of, see

https://github.com/mne-tools/mne-python/blob/da9cd19a9b65e792e2523f159820549ec6082f9d/doc/conf.py#L531-L616

for an example. We can also ignore nitpicky warnings using nitpick_ignore, but the normal warnings can't be selectively ignored (suppress_warnings ignores warning categories).

@keewis
Copy link
Collaborator

keewis commented Dec 10, 2019

looks like I was wrong, numpydoc does not have much to do with this. The real culprit is napoleon: see sphinx-doc/sphinx#6861

@keewis
Copy link
Collaborator

keewis commented Dec 17, 2019

this is not yet fully fixed, we still need to silence the nitpick warnings. That is blocked by the napoleon bug, though.

@keewis keewis reopened this Dec 17, 2019
@max-sixty
Copy link
Collaborator

@keewis I'm guessing we're still stuck on this because of the Napoleon bug?

Your efforts to quash the previous warnings are laudable, and would be great if we can add tests to ensure they don't creep back in...

@DocOtak
Copy link
Contributor

DocOtak commented Mar 4, 2020

Every time I see activity on this... I feel like it's all my fault. Feel free to undo whatever is needed.

@keewis
Copy link
Collaborator

keewis commented Mar 4, 2020

most warnings are due to the bug, yes. The others are broken references to accessor methods like Dataset.plot.scatter (see #3625). Last time I checked we did not have new warnings unrelated to the bug, but my grep regexp might have missed a few lines.

We could do something to make sure we don't introduce more warnings, though: the nitpicky mode allows ignoring warnings using nitpick_ignore, so we might be able to ignore every nitpicky warning related to docstrings / napoleon. We still would need to fix or bypass #3625 first, though.

@keewis
Copy link
Collaborator

keewis commented Jul 25, 2020

I've been working on a type preprocessor for napoleon (sphinx-doc/sphinx#7690) which is pretty close (to be included in sphinx v3.2.0). Hopefully that will silence all those warnings and also fix most of the broken type links.

Edit: the PR has been merged, so now we only have to wait until the next release of sphinx. That feature is pretty strict about the format of the type spec, so I'll create a PR to update our docstrings.

@keewis keewis mentioned this issue Jul 29, 2020
4 tasks
@stale
Copy link

stale bot commented Apr 17, 2022

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

@stale stale bot added the stale label Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools stale topic-documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants