sphinx-doc / sphinx

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

Type hints are irregularly documented (+link generation broken) #9813

Open gschwaer opened 2 years ago

gschwaer commented 2 years ago

Describe the bug

Sphinx processes and outputs type hints differently to html depending on

In some cases the full canonical path including all levels of module path is shown (like: module.Name), some others show only the class name (like: Name). Some of the combinations also break links.

Here is what I found:

                                | __future__.annotations  | no __future__.annotations |
                                |  unquoted   |  quoted   |  unquoted    |   quoted   |
--------------+-----------------+-------------+-----------+--------------+------------|
plain import  | module function |    F, L     |   F, L    |     F, L     |    F, L    |
              | class attribute |    F, L     |   F, L    |     F, L     |    F, L    |
              | class property  |    F, L     |   F, L    |     F, L     |    F, L    |
              | class method    |    F, L     |   F, L    |     F, L     |    F, L    |
from import   | module function |    F, L     |   C, L    |     F, L     |    F, L    |
              | class attribute |    F, L     |   C, N    |     F, L     |    F, L    |
              | class property  |    F, L     |   C, N    |     F, L     |    F, L    |
              | class method    |    F, L     |   C, L    |     F, L     |    F, L    |
type checking | module function |    C, L     |   C, L    |     n/a      |    C, L    |
              | class attribute |    C, N     |   C, N    |     n/a      |    C, N    |
              | class property  |    C, N     |   C, N    |     n/a      |    C, N    |
              | class method    |    C, L     |   C, L    |     n/a      |    C, L    |

F == full path, C == class-only, L == link works, N == no link, n/a == does not apply (__future__.annotations is required)

Observations:

Note: Showing classes-only is really neat (especially for small projects) as it is more concise. But it would be best if this could be configured (enabled/disabled).

How to Reproduce

$ git clone https://github.com/gschwaer/sphinx_test/
$ cd some_project
$ git checkout autodoc_irregular_type_resolving
$ python3 -m venv .venv
$ pip install -r requirements.txt
$ make
$ # open _build/html/index.html

Check out Test.py which has the include of __future__.annotations at the top.

Expected behavior

Your project

https://github.com/gschwaer/sphinx_test/tree/autodoc_irregular_type_resolving

Screenshots

Example showing the case where types are shown as full canonical path and links work (with future annotations, from-import schema, type hints not quoted):

image

Example showing the case where types are shown as class-only and some links are not generated (the generated links work) (with future annotations, from-import schema, quoted type hints):

image

OS

Linux

Python version

3.8.10

Sphinx version

4.2.0

Sphinx extensions

sphinx.ext.autodoc

Extra tools

No response

Additional context

I am using RTD theme because alabaster is really not helping to showcase this issue. The effects (class-only vs full path and missing links) are the same for both themes.

cjw296 commented 1 year ago

I'm hitting this too with this source:

from typing import Callable, TYPE_CHECKING, Iterable, TypeVar, Optional

if TYPE_CHECKING:
    from .example import Example

#: This is an evaluator
Evaluator = Callable[['Example'], Optional[str]]

Which is hit by autodoc from this:

.. automodule:: sybil.typing
  :members: Evaluator

Example is not rendered as a link in type annotation. If I change the Python source do this, the link is correctly rendered:

from typing import Callable, TYPE_CHECKING, Iterable, TypeVar, Optional

if TYPE_CHECKING:
    from .example import Example

#: This is an evaluator
Evaluator = Callable[['sybil.example.Example'], Optional[str]]

...but is that a valid type definition?

I'm also on Sphinx 4.2.0.

provinzkraut commented 1 year ago

@gschwaer Did you ever find something like a temporary workaround for this?

I am currently trying to migrate an existing codebase to Sphinx and am receiving thousands of warnings of broken links because of this, since the library makes extensive use of the TYPE_CHECKING import.

gschwaer commented 1 year ago

@provinzkraut Sorry, I did not. Back when I reported the issue I took a quick look into the Sphinx code to see if I could fix it myself. But the code was very confusing to me, so I gave up and reported it here. For the project I was working on, it was a hassle, but no deal breaker, so we just left it as ugly as it was.

If you by chance find a solution yourself (or a better suited framework) please post it here. Thx

cjw296 commented 1 year ago

Here's another example of Sphinx doing really badly on this: https://testfixtures.readthedocs.io/en/latest/api.html#testfixtures.compare

Here's the code: https://github.com/simplistix/testfixtures/blob/e201cb48097b358b4acee1d9cc8c3ee46c14abe0/docs/api.txt#L1-L10 https://github.com/simplistix/testfixtures/blob/e201cb48097b358b4acee1d9cc8c3ee46c14abe0/testfixtures/comparison.py#L660-L677

cjw296 commented 1 year ago

I know there have been changes in this area recently, but even on the last version, none of the types in the parameters are being linked:

https://output.circle-artifacts.com/output/job/f0253560-91f6-4749-8622-ccead12c5e78/artifacts/0/docs/api.html

Screenshot 2023-01-08 at 14 31 56
provinzkraut commented 1 year ago

@AA-Turner Could you explain why this was added to "some future version"? That's a bit of a blow honestly since it means autodoc (and therefore Sphinx) is not actually usable for projects that wish to derive their documentation from type hints, and probably won't be for the foreseeable future.

electric-coder commented 1 year ago

@provinzkraut the links can be made to work (in the cases where autodoc doesn't resolve them automatically) by coding them explicitly into the .rst files or the type fields of the docstrings using correct cross-reference syntax. There are currently only 2 exceptions that don't allow the workaround, one being the yield type #10982 and the other the overload #10436 . The examples shown in this thread can be resolved by using the said workaround so these aren't "breaking bugs" but they do require extra work.

provinzkraut commented 1 year ago

@electric-coder

by coding them explicitly into the .rst files or the type fields of the docstrings using correct cross-reference syntax

the examples shown in this thread can be resolved by using the said workaround so these aren't "breaking bugs" but they do require extra work.

They are "breaking bugs" because the feature being talked about here does not work correctly. The fact that you can get the desired result in a different way, simply by not using this feature doesn't change that.

This issue is specifically about type hints and the automatic documentation of them. Saying "it works if you don't document these things with type hints" doesn't address this issue.

The feature is broken in its current state as it does not work as advertised in a way that's usable for a larger code base. Even the workaround you mention isn't really usable, because if you were to do that, you'd end up with an inconsistent docstring format, which not only is confusing and requires extra attention, but can also throw linters of if you configure them to enforce a particular format.

AA-Turner commented 1 year ago

@AA-Turner Could you explain why this was added to "some future version"? That's a bit of a blow honestly since it means autodoc (and therefore Sphinx) is not actually usable for projects that wish to derive their documentation from type hints, and probably won't be for the foreseeable future.

Hi Janek,

This is as I don't have time to work on this, and I don't want to offer false hope of a quick fix. As with everything--constrained resources (especially in voluntary work &c. &c.)! I'm very happy to review PRs though, if you would point me in their direction.

Thanks, Adam

provinzkraut commented 1 year ago

@AA-Turner

Hey Adam, thanks for the explanation! That's certainly a valid reason.

and I don't want to offer false hope of a quick fix

I have since spent a good amount of time on this effort and realised myself that this will be anything but a quick fix. There are a lot of open issues related to how autodoc deals with type annotations, and most of them boil down to what has been outlined here.

The only way to properly solve this is to fundamentally change how autodoc resolves with type hints, and create a consistent resolution. I have had some partial success with an AST-based approach, as did the excellent sphinx-resolve-py-references extension, and I believe this would be the way to go.

A merge of this into Sphinx has been discussed in https://github.com/sphinx-doc/sphinx/issues/4961, but has not progressed as of today. I am not sure why that is, but I'd be willing to help move this along if actionable blockers could be identified.

Maybe it would be worth tracking this in a separate issue?

electric-coder commented 9 months ago

Another case that hasn't been mentioned: When you use from __future__ with a quoted class that's defined in the same module (e.g. you type hint the class above and it's defined further down in the module) the link works but the type is rendered with the fully qualified name.

picnixz commented 8 months ago

So I hit this one when documenting the testing module. I think the next thing to do (after fixing the testing module and Windows tests) is to address this issue. The issue I faced is inside a signature and this is not handled by autodoc_type_aliases.

electric-coder commented 8 months ago

@picnixz this is likely the one issue that's affecting most docs in the wild at the moment, together with some types not linking properly (like None, Yield and Literal). I mean, except for the rare user who's willing to write a custom extension and process all those type links this set of bugs is all across the ecosystem's docs right now.

e-gebes commented 1 month ago

I ran into a similar issue with dataclasses and TYPE_CHECKING, basically my code is:

from __future__ import annotations

if TYPE_CHECKING:
    from foo.bar import Engine

@dataclass
class Machine:
    engine: Engine

And I get a more than one target found for cross-reference error, because a second class with that name is defined somewhere else in the project.

I try to workaround this issue by importing differently such that the type hint "string" itself becomes unambiguous: from foo import bar and engine: bar.Engine.

Any progress on the issue is appreciated!