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

Warnings raised on variable and attribute type annotations #7808

Closed
MarkusH opened this issue Jun 9, 2020 · 14 comments
Closed

Warnings raised on variable and attribute type annotations #7808

MarkusH opened this issue Jun 9, 2020 · 14 comments

Comments

@MarkusH
Copy link

MarkusH commented Jun 9, 2020

Describe the bug

autodoc signature for non-builtin types raises warning and thus fails nitpicking:

/path/to/foo.py:docstring of foo.Foo.a:: WARNING: py:class reference target not found: Optional[str]

To Reproduce

Steps to reproduce the behavior:

Create a file foo.py with the following content:

from typing import Optional


class Foo:
    a: Optional[str] = None

Use sphinx-apidoc to generate an rst file, while enabling autodoc and intersphinx: sphinx-apidoc --ext-autodoc --ext-intersphinx

Make sure the intersphinx_mapping in the Sphinx conf.py contains "python": ("https://docs.python.org/3.8/", None),

Run make html with loud warnings and nitpicking: SPHINXOPTS="-n -v -W --keep-going" make html.

You will get an error message

/path/to/foo.py:docstring of foo.Foo.a:: WARNING: py:class reference target not found: Optional[str]

Expected behavior

I'd expect Sphinx to resolve the type annotation Optional[str] and possibly link both classes.

Environment info

  • OS: Linux
  • Python version: 3.8.3
  • Sphinx version: 3.1.0
  • Sphinx extensions: sphinx.ext.autodoc, sphinx.ext.intersphinx

Additional context

I think the issue stems from the change in 88e8ebb which appears to try to lookup the entire type annotation as a single class.

Using _parse_annotation() instead of type_to_xref() solves this particular issue:

diff --git a/sphinx/domains/python.py b/sphinx/domains/python.py
index fc1136ae2..6101de56a 100644
--- a/sphinx/domains/python.py
+++ b/sphinx/domains/python.py
@@ -623,7 +623,7 @@ class PyVariable(PyObject):
 
         typ = self.options.get('type')
         if typ:
-            signode += addnodes.desc_annotation(typ, '', nodes.Text(': '), type_to_xref(typ))
+            signode += addnodes.desc_annotation(typ, '', nodes.Text(': '), *_parse_annotation(typ))
 
         value = self.options.get('value')
         if value:
@@ -868,7 +868,7 @@ class PyAttribute(PyObject):
 
         typ = self.options.get('type')
         if typ:
-            signode += addnodes.desc_annotation(typ, '', nodes.Text(': '), type_to_xref(typ))
+            signode += addnodes.desc_annotation(typ, '', nodes.Text(': '), *_parse_annotation(typ))
 
         value = self.options.get('value')
         if value:

However, it doesn't seem to work with custom classes. Take this snippet for example:

class Bar:
    i: int


class Foo:
    a: Bar

This causes the following warning:

foo.py:docstring of foo.Foo.a:: WARNING: py:class reference target not found: Bar
MarkusH added a commit to crate/crate-operator that referenced this issue Jun 9, 2020
MarkusH added a commit to crate/crate-operator that referenced this issue Jun 9, 2020
@pquentin
Copy link

pquentin commented Jun 9, 2020

We have a similar problem in the Trio project where we use annotations like "str or list", ThisType or None" or even "bytes-like" in a number of place. Here's an example: https://github.com/python-trio/trio/blob/dependabot/pip/sphinx-3.1.0/trio/_subprocess.py#L75-L96

@njsmith
Copy link
Contributor

njsmith commented Jun 9, 2020

To clarify a bit on the Trio issue: we don't expect sphinx to magically do anything with bytes-like, but currently you can use something like this in a Google-style docstring:

    Attributes:
      args (str or list): The ``command`` passed at construction time,
          specifying the process to execute and its arguments.

And with previous versions of Sphinx, it renders like this:

image

https://trio.readthedocs.io/en/v0.15.1/reference-io.html#trio.Process.args

Notice that str and list are both hyperlinked appropriately.

So Sphinx used to be able to cope with this kind of "or" syntax, and if it can't anymore it's a regression.

@pckroon
Copy link

pckroon commented Jun 10, 2020

This also occurs with built-in container classes and 'or'-types (in nitpick mode):

WARNING: py:class reference target not found: tuple[str]
WARNING: py:class reference target not found: str or None

Unfortunately this breaks my CI pipeline at the moment. Does anyone know a work-around other than disabling nitpick mode?

@tk0miya
Copy link
Member

tk0miya commented Jun 10, 2020

Thank you for reporting. I made a PR #7814 based on your patch.
@njsmith Could you confirm this resolves trio's case?

@pquentin
Copy link

Unfortunately even with #7814 I still see the same warning: trio/__init__.py:docstring of trio.Process::py:class reference target not found: str or list.

If you want to try it out, we run sphinx-build -nW -b html source build in the docs/ folder after having installed docs-requirements.txt.

Is there anything we can do to help here?

@tk0miya
Copy link
Member

tk0miya commented Jun 11, 2020

Hmm... the trio's case came from #7583.
@eric-wieser I'd like to revert it on 3.1.x (and reconsider it for 3.2 release again). What do you think?

tk0miya added a commit that referenced this issue Jun 11, 2020
Fix #7808: autodoc: Warnings raised on variable and attribute type annotations
@eric-wieser
Copy link
Contributor

eric-wieser commented Jun 11, 2020

Instead of reverting it, can we just silence the warning, and revert to the old behavior if parsing fails?

@tk0miya
Copy link
Member

tk0miya commented Jun 11, 2020

It can suppress a part of nitpicky warnings. But numpydoc allows some kinds of python-valid type descriptions: int, optional (valid tuple) and `array_like' (valid name). The rule of the parameter type of numpydoc is "be as precise as possible". If my understanding is correct, no way to distinguish PEP-484 based type annotation or numpydoc.

@eric-wieser
Copy link
Contributor

Perhaps then :type: ... markers should always allow non-pep484 descriptions without a warning?

@tk0miya
Copy link
Member

tk0miya commented Jun 11, 2020

I don't have idea to realize it now. I'll try it tomorrow. Please let me know if you have idea.

@tk0miya
Copy link
Member

tk0miya commented Jun 13, 2020

Finally, I can't find the way to skip conversion for non-pep484 descriptions. So I'll revert it from 3.1.x. Let's reconsider it again for 3.2.0.

@pquentin
Copy link

Thanks! 3.1.1 fixed the issue.

@MarkusH
Copy link
Author

MarkusH commented Jun 15, 2020

Thank you for the quick resolution! 🙂

@pckroon
Copy link

pckroon commented Aug 10, 2020

This issue happens again on 3.2.0. It works on 3.1.2.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants