search

sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.46k stars 2.1k forks source link

Show docstring on error #11924

Open astrojuanlu opened 8 months ago

astrojuanlu commented 8 months ago

Is your feature request related to a problem? Please describe. I'm always frustrated when Sphinx complains about a problem in a docstring, and then doesn't show me said docstring. For example:

writing output... [ 50%] api/kedro_datasets.networkx.GraphMLDataset .. api/kedro_datasets.pandas.SQLQueryDataset

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/sphinx/cmd/build.py", line 281, in build_main
    app.build(args.force_all, args.filenames)
  File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/sphinx/application.py", line 343, in build
    self.builder.build_all()
...
  File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/logging/__init__.py", line 806, in filter
    result = f.filter(record)
  File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/sphinx/util/logging.py", line 426, in filter
    raise exc
sphinx.errors.SphinxWarning: /home/runner/work/kedro-plugins/kedro-plugins/kedro-datasets/kedro_datasets/tracking/json_dataset.py:docstring of kedro_datasets.tracking.json_dataset.JSONDataset:1:py:func reference target not found: typing.NewType

Warning, treated as error:
/home/runner/work/kedro-plugins/kedro-plugins/kedro-datasets/kedro_datasets/tracking/json_dataset.py:docstring of kedro_datasets.tracking.json_dataset.JSONDataset:1:py:func reference target not found: typing.NewType
make: *** [Makefile:91: rtd] Error 2
Error: Process completed with exit code 2.

but JSONDataset has no trace of typing.NewType. It's probably coming from some transformation (sphinx.ext.napoleon, sphinx-autodoc-typehints, ...) or from an inherited member (?) but what am I supposed to do here?

Describe the solution you'd like If at least the docstring was shown, I would have more tools to try to solve the issue.

Like:

Warning, treated as error:
/home/runner/work/kedro-plugins/kedro-plugins/kedro-datasets/kedro_datasets/tracking/json_dataset.py:docstring of kedro_datasets.tracking.json_dataset.JSONDataset:1:py:func reference target not found: typing.NewType

Source docstring:

"""
Return a list containing n references to x.

:param kind: Optional "kind" of ingredients.
:type kind: typing.NewType or None
            ^^^^^^^^^^^^^^
"""

(extra points if the ^^^^ underline is included!)

Describe alternatives you've considered This, but behind a -v/--verbose or --debug flag.

astrojuanlu commented 8 months ago

Related: https://github.com/sphinx-doc/sphinx/issues/2953

electric-coder commented 8 months ago

but what am I supposed to do here?

Is that the docstring causing the error? Well Ctrl+F with typing.NewType should allow to narrow it down. If it's not a standard library type it's either something you implemented or imported.

You already got a +8 (in 2 hours) so I'm going to downvote for at least someone to be in disagreement.

If you're using the 3rd party sphinx-autodoc-typehints you're generally saving yourself the trouble of writing types into docstrings to being with. There's where the problem starts: how can you be sure the error isn't being raised internally by the 3rd party extension in an intermediate step of the build process? I'd love to use that extension, but since it comes with its own list of bugs and it doesn't support the modern pipe union types, you have to think twice before piling another extension into your workflow stack - because that dependency comes with consequences.

astrojuanlu commented 8 months ago

Hi @electric-coder , I'm going to kindly suggest that you read what I wrote:

but JSONDataset has no trace of typing.NewType. It's probably coming from some transformation (sphinx.ext.napoleon, sphinx-autodoc-typehints, ...) or from an inherited member (?)

Python docstrings that contain arbitrary Sphinx directives can insert arbitrary nodes, plus plugins can transform the docstring itself too. So your Ctrl+F suggestion does not apply.

You already got a +8 (in 2 hours) so I'm going to downvote for at least someone to be in disagreement.

As per the number of upvotes, for full disclosure I posted this in a Slack channel where there are many other people that have suffered this problem and I encouraged them to upvote.

There's where the problem starts: how can you be sure the error isn't being raised internally by the 3rd party extension in an intermediate step of the build process?

Happy to see we all want better debugging information in Sphinx.

I'd love to use that extension, but since it comes with its own list of bugs and it doesn't support the modern pipe union types, you have to think twice before piling another extension into your workflow stack - because that dependency comes with consequences.

I'm not sure if this is advice for me, but my policy of choosing dependencies is completely unrelated to the question on how to debug docstring problems.

Your hostility is very puzzling, but if you think I'm new to open source in general or Sphinx in particular, maybe you can do a bit of research.

picnixz commented 8 months ago

I assume that you are using autodoc for that one. If not, forget what I'll be saying.

The reference that is not found is either due to:

As such, it's extremely difficult to actually pinpoint the exact location of the error as you've observed. The only thing we can do is saying "there is an issue when we wanted to generated the node corresponding to this specific object".

Now, the warning is actually emitted during the resolution phase which occurs waaaaay later docstrings have been processed (informally we have parse docstrings -> generate rst -> inject the rst content -> work on the extended RST document). In fact, I don't think we keep track of the 'source' that was generated by autodoc and we only "show" it when with debug verbosity.

At some point, I wanted to store the RST content generated by autodoc as well because 1) it could help see what is used 2) use it as a 'template' for a better customization (namely, I would essentially run autodoc to generate the RST content and then I would "fix" it to fit my needs). This could technically help you because you will see what's being parsed.

Unfortunately, just rendering the docstring on error won't help you. However, rendering the RST that was used might help you although the RST content could be transformed inbetween and you would still not see anything relevant. For instance:

  1. Parse docstring
  2. Generate RST
  3. Generate doctree (the nodes)
  4. Resolve references
  5. Yay

would be the "vanilla" flow. However, you could have

3a. Generate doctree 3b. Change a bit the doctree by adding some new reference nodes

  1. Resolve references. Oh no, there are somewhat bad reference nodes that can't be resolved :(
  2. not yay

The issue now is that the RST content is no more responsible for the warning but something else is. And then you would be (also) misguided in thinking that the issue was with the docstring / autodoc / RST.

In summary, I cannot guarantee that showing a docstring would help you more. What I'm pretty sure about however is that if we were to show it, then we would likely confuse users (although we are already confusing them). I think it's misleading to say that the error occurs in the docstring and we should rather say that "generating this object caused this error" (still you won't get any better idea on how to fix it, I agree).

Now, on the good news: we are planning to redo autodoc at some point (maybe this year, next year I don't know but it's something we need to do). The reason is that we currently use dynamic analysis for generating things. While this is quite efficient, there are many false positives or corner-cases due to how objects are represented in Python at runtime (e.g., TypeVars are currently badly documented and when they appear in a type annotation, they are not correctly resolved). So, I think we could avoid having those kind of issues where the error message is more confusing than helping if we were to fix in the first place how references are generated and resolved (which is a real challenge !).

astrojuanlu commented 8 months ago

Thanks a lot for the detailed answer @picnixz !