Skip to content

Google style docstrings throw warning target not found: optional #11376

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

Open
RemDelaporteMathurin opened this issue Apr 27, 2023 · 6 comments
Open

Comments

@RemDelaporteMathurin
Copy link

Describe the bug

When trying to document a function with Google Style docstrings, I get the following warning:

 WARNING: py:class reference target not found: optional

I think "optional" is identified as a type, when it shouldn't.

Running Sphinx v6.2.1
loading pickled environment... failed
failed: Can't get attribute '_stable_repr_object' on <module 'sphinx.builders.html' from '/usr/local/python/3.10.4/lib/python3.10/site-packages/sphinx/builders/html/__init__.py'>
loading intersphinx inventory from https://docs.python.org/3.7/objects.inv...
building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 1 source files that are out of date
updating environment: [new config] 1 added, 0 changed, 0 removed
reading sources... [100%] index                                                                                                                                                                                          
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] index                                                                                                                                                                                           
/workspaces/h-transport-materials/mwe/mycode.py:docstring of mycode.my_func:1: WARNING: py:class reference target not found: optional
generating indices... genindex done
writing additional pages... search done
copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 1 warning.

The HTML pages are in _build/html.

How to Reproduce

conf.py

import sys

sys.path.insert(0, ".")
extensions = ["sphinx.ext.autodoc", "sphinx.ext.napoleon", "sphinx.ext.intersphinx"]
napoleon_google_docstring = True
intersphinx_mapping = {"python": ("https://docs.python.org/3.7", None)}
nitpicky = True

mycode.py

def my_func(arg1, arg2=2.0):
    """_summary_

    Args:
        arg1 (int): _description_
        arg2 (float, optional): _description_. Defaults to 2.0.
    """
    pass

index.rst

 .. autofunction:: mycode.my_func

Environment Information

Platform:              linux; (Linux-5.4.0-1105-azure-x86_64-with-glibc2.31)
Python version:        3.10.4 (main, Apr  3 2023, 22:35:52) [GCC 9.4.0])
Python implementation: CPython
Sphinx version:        6.2.1
Docutils version:      0.19
Jinja2 version:        3.1.2
Pygments version:      2.14.0

Sphinx extensions

No response

Additional context

No response

@picnixz
Copy link
Member

picnixz commented Apr 29, 2023

It appears that using :obj:`float` would solve the issue.

Now, what is weird is that the reST lines contain :type arg2: :obj:`float`, optional or :type arg2: float, optional. So the issue is not from the Napoleon parser.

After some digging, you get that reST document:

<literal_strong>
  x (
<pending_xref py:class="True" py:module="testmodule" refdomain="py" refexplicit="False" refspecific="True" reftarget="int" reftype="class">
  <literal_emphasis>
    int
<literal_emphasis>
  , 
<pending_xref py:class="True" py:module="testmodule" refdomain="py" refexplicit="False" refspecific="True" reftarget="optional" reftype="class">
  <literal_emphasis>
    optional
)
<literal_strong>
  y (
<pending_xref py:class="True" py:module="testmodule" refdoc="index" refdomain="py" refexplicit="False" reftarget="int" reftype="class" refwarn="False">
  <literal classes="xref py py-class">
    int, optional
)

In other words, the handler for type fields considers int, optional as a list of types but with :obj:int, optional, the reference is only using int and not both. I will try to fix that this afternoon.

@AA-Turner AA-Turner added this to the some future version milestone Apr 29, 2023
@RemDelaporteMathurin
Copy link
Author

Awesome, let me know if I can help!

@picnixz
Copy link
Member

picnixz commented May 1, 2023

Well, actually... I totally forgot about this ! I'll be busy this week but I'll probably have time this week-end. If you want to help / contribute, I'd suggest debugging Sphinx or directly by cloning this repository, modifying the sources, creating a fake test and printing things here and there (only works if the issue is Sphinx related). If this is a docutils issue, digging into the codesource is likely to be complex.

@RemDelaporteMathurin
Copy link
Author

RemDelaporteMathurin commented May 2, 2023

Oh I'm afraid I'm not familiar enough with Sphinx to give useful insights. I'm unlikely to get some bandwith to have a look at it soon anyway.

Also, I don't know where this warning is triggered in the source code so it's kinda hard to know where to start

@picnixz
Copy link
Member

picnixz commented May 5, 2023

Since you are in nitpicky mode, the warning is emitted by the ReferencesResolver in the warn_missing_references method. But this is not really important because it is a post-transformation, so it happens after the nodes are created (i.e., after pending_xref nodes are created).

We need to find which transformation is responsible for creating those pending_xref nodes. The domain (here, Python) has directives (e.g., py:function) which allow some "doc fields" (e.g., :param [...]:). One of these fields is the "parameter" field which can be paired with the "parameter type" field, namely, when you find

:param x: [...]
:type x: [...]

you can think of this as a single group for a parameter and its type. This is represented by sphinx.domains.python.PyTypedField. Now, this field object is actually responsible for parsing and creating the corresponding docutils nodes (in our case, the pending_xref node will be created). Assume that you have the following:

def func(x, y=2, z=3):
    """_summary_

    Args:
        x (int): the x
        y (int, optional): the y
        z (:obj:`int`, optional): the z
    """
    pass

Then, the Napoleon parser transforms the docstring into:

def func(x, y=2, z=3):
    """_summary_

    :param x: the x
    :type x: int
    :param y: the y
    :type y: int, optional
    :param z: the z
    :type z: :obj:`int`, optional
    """
    pass

Now, the important transformation is done by ObjectDescription.run which will create the nodes. At some point, we have:

<desc classes="py function" desctype="function" domain="py" nocontentsentry="False" noindex="False" noindexentry="False" objtype="function">
  <desc_signature _toc_name="func()" _toc_parts="('testmodule', 'func')" class="" classes="sig sig-object" fullname="func" ids="testmodule.func" module="testmodule">
    <desc_addname classes="sig-prename descclassname" xml:space="preserve">
      testmodule.
    <desc_name classes="sig-name descname" xml:space="preserve">
      func
    <desc_parameterlist xml:space="preserve">
      <desc_parameter xml:space="preserve">
        <desc_sig_name classes="n">
          x
      <desc_parameter xml:space="preserve">
        <desc_sig_name classes="n">
          y
        <desc_sig_operator classes="o">
          =
        <inline classes="default_value" support_smartquotes="False">
          2
      <desc_parameter xml:space="preserve">
        <desc_sig_name classes="n">
          z
        <desc_sig_operator classes="o">
          =
        <inline classes="default_value" support_smartquotes="False">
          3
  <desc_content>

Then, nested_parse_with_titles in ObjectDescription.run is called and the <desc_content> is populated as follows:

<desc_content>
  <paragraph>
    _summary_
  <field_list>
    <field>
      <field_name>
        param x
      <field_body>
        <paragraph>
          the x
    <field>
      <field_name>
        type x
      <field_body>
        <paragraph>
          int
    <field>
      <field_name>
        param y
      <field_body>
        <paragraph>
          the y
    <field>
      <field_name>
        type y
      <field_body>
        <paragraph>
          int, optional
    <field>
      <field_name>
        param z
      <field_body>
        <paragraph>
          the z
    <field>
      <field_name>
        type z
      <field_body>
        <paragraph>
          <pending_xref py:class="True" py:module="testmodule" refdoc="index" refdomain="py" refexplicit="False" reftarget="int" reftype="obj" refwarn="False">
            <literal classes="xref py py-obj">
              int
          , optional

Now:

  • when the reST parser encounters :type x: int or :type y: int, optional, it simply parses without dispatching to some special role handler. And this is the way it should be.
  • if it encounters :type z: :obj:`int`, optional, it sees :obj:`int`, which is the usage of a role (here the obj role) and thus asks the corresponding role to create the node. This is the responsibility of PyXRefRole to do so and this is at the moment (i.e., just after the role function has been executed) that the pending_xref node is created.

After that, we call DocFieldTransformer.transform_all and this will be inside this transformation that we will have a reference to optional that is created. It is created in TypedField.make_field. One of the inputs is:

{
  'x': [<#text: 'int'>], 
  'y': [<#text: 'int, optional'>], 
  'z': [<pending_xref: <literal...>>, <#text: ', optional'>]}
}

Then, you have this:

def handle_item(fieldarg: str, content: str) -> nodes.paragraph:
par = nodes.paragraph()
par.extend(self.make_xrefs(self.rolename, domain, fieldarg,
addnodes.literal_strong, env=env))
if fieldarg in types:
par += nodes.Text(' (')
# NOTE: using .pop() here to prevent a single type node to be
# inserted twice into the doctree, which leads to
# inconsistencies later when references are resolved
fieldtype = types.pop(fieldarg)
if len(fieldtype) == 1 and isinstance(fieldtype[0], nodes.Text):
typename = fieldtype[0].astext()
par.extend(self.make_xrefs(self.typerolename, domain, typename,
addnodes.literal_emphasis, env=env,
inliner=inliner, location=location))
else:
par += fieldtype
par += nodes.Text(')')
par += nodes.Text(' -- ')
par += content
return par

Here, the make_xrefs will process 'int' and 'int, optional' but not the type for z. The reason is because of len(fieldtype) == 1 and isinstance(fieldtype[0], nodes.Text). More precisely, when processing 'int' and 'int, optional', it will actually use PyXRefMixin.make_xrefs instead of the default implementation of TypedField:

def make_xrefs(
self,
rolename: str,
domain: str,
target: str,
innernode: type[TextlikeNode] = nodes.emphasis,
contnode: Node | None = None,
env: BuildEnvironment | None = None,
inliner: Inliner | None = None,
location: Node | None = None,
) -> list[Node]:
delims = r'(\s*[\[\]\(\),](?:\s*o[rf]\s)?\s*|\s+o[rf]\s+|\s*\|\s*|\.\.\.)'
delims_re = re.compile(delims)
sub_targets = re.split(delims, target)
split_contnode = bool(contnode and contnode.astext() == target)
in_literal = False
results = []
for sub_target in filter(None, sub_targets):
if split_contnode:

And this is where the thing gets bad. Recall that we are in the context that the node is a single text node, e.g., 'int, optional'. This single text node is then interpreted according to some separator logic. Long story short,

delims = r'(\s*[\[\]\(\),](?:\s*o[rf]\s)?\s*|\s+o[rf]\s+|\s*\|\s*|\.\.\.)'

tells you that if you put something like list of int or str or list[str], then you extract list and int (resp. str, list, and str) consider the of (resp. or and the brackets []) to be "inactive" (i.e., non-types). Everything else is considered a type and a pending_xref node will be created. Since optional could be a type name, it is not considered inactive, hence we have the issue.

I cannot tell for the other domains, but here is what I would do for Python:

  • if the text ends with , optional, then it will ignored. This is to avoid the situation where the user defined a type called optional and use as list of optional or list[tuple[int, optional]]. For tuples, you would have (int, optional) (so there would be some closing delimiter). For unions, you would use | or or (I don't think the comma would be use).

  • The only situation I can think of where , optional can actually refer to a real type optional is when you are doing some "type constraints", e.g.,:

    Args: 
      x (T, optional): The x is an instance of T *and* and instance of optional
    

    However, it would be completely confusing if you were to read it normally so I think it should be safe to say that a type string that is entirely textual and ends with , optional should not create a reference to optional.

  • Note that the behaviour only occurs in nitpicky mode. In regular mode, this is hidden and users are kind of lied to since they did not know that a reference was actually created at some point.

@electric-coder
Copy link

electric-coder commented Jan 3, 2024

Google Style docstrings

WARNING: (...) target not found: optional

I tried searching in the Google Python Style Guide and there's no mention of using the trailing optional so an argument can be made that it shouldn't be used and "not found" is the correct action by Sphinx.

Args: 
  x (T, optional):

jcfr added a commit to jcfr/idc-index that referenced this issue May 17, 2024
Remove the use of the "optional" hint from the docstrings to address the
following Sphinx warnings. The "optional" hint is not supported by Sphinx
and was inconsistently specified for keyword arguments with default values.

```
[...]
/path/to/index.py:docstring of idc_index.index.IDCClient.download_from_selection:1: WARNING: py:class reference target not found: optional
/path/to/index.py:docstring of idc_index.index.IDCClient.get_dicom_series:1: WARNING: py:class reference target not found: optional
/path/to/index.py:docstring of idc_index.index.IDCClient.get_dicom_studies:1: WARNING: py:class reference target not found: optional
/path/to/index.py:docstring of idc_index.index.IDCClient.get_patients:1: WARNING: py:class reference target not found: optional
[...]
```

For more details, see the related Sphinx issue:
* sphinx-doc/sphinx#11376 (Google style
  docstrings throw warning target not found: optional)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants