sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

Multiple return types aren't hyperlinked

Open mhostetter opened this issue 4 years ago • 15 comments

Using multiple return values, sphinx/napoleon doesn't hyperlink the return types as it does with parameters. I tried with and without explicitly naming the return values.

  • OS: Ubuntu 20.04
  • Python version: 3.8.10
  • Sphinx version: 4.0.2
  • Sphinx extensions: ['sphinx.ext.napoleon', 'sphinx.ext.mathjax']
    r"""
    Parameters
    ----------
    n : int
        A positive integer.
    B : int, optional
        The max divisor in the trial division. The default is `None` which corresponds to :math:`B = \sqrt{n}`.
        If :math:`B > \sqrt{n}`, the algorithm will only search up to :math:`\sqrt{n}`, since a factor of :math:`n`
        cannot be larger than :math:`\sqrt{n}`.

    Returns
    -------
    list
        The discovered prime factors :math:`\{p_1, \dots, p_k\}`.
    list
        The corresponding prime exponents :math:`\{e_1, \dots, e_k\}`.
    int
        The residual factor :math:`n_r`.
    """

renders like this

image

And when naming the return values

    r"""
    Parameters
    ----------
    n : int
        A positive integer.
    B : int, optional
        The max divisor in the trial division. The default is `None` which corresponds to :math:`B = \sqrt{n}`.
        If :math:`B > \sqrt{n}`, the algorithm will only search up to :math:`\sqrt{n}`, since a factor of :math:`n`
        cannot be larger than :math:`\sqrt{n}`.

    Returns
    -------
    p : list
        The discovered prime factors :math:`\{p_1, \dots, p_k\}`.
    e : list
        The corresponding prime exponents :math:`\{e_1, \dots, e_k\}`.
    n_r : int
        The residual factor :math:`n_r`.
    """

it renders like this

image

However, when using a single return value, the hyperlinks work as expected.

image

FYI, I am using intershpinx_mapping to link to the python docs, which correctly works for the Parameters section.

# File: conf.py
intersphinx_mapping = {
    'python': ('https://docs.python.org/3', None),
    'numpy': ('https://numpy.org/doc/stable/', None),
    'numba': ('https://numba.pydata.org/numba-doc/latest/', None)
}

mhostetter avatar Jul 01 '21 03:07 mhostetter

On one hand, I have been a victim of this myself and your markup looks correct to me, but on the other hand I just checked that for example the Astropy folks got it working https://docs.astropy.org/en/latest/api/astropy.coordinates.Attribute.html#astropy.coordinates.Attribute.convert_input so I'm not sure what is happening here 🤔

astrojuanlu avatar Jul 01 '21 15:07 astrojuanlu

FWIW, from my example, this is the project, conf.py, example function, and the rendered docs.

I'm using sphinx_rtd_theme and astropy isn't. I don't know if that makes a difference.

mhostetter avatar Jul 01 '21 15:07 mhostetter

Could you try napoleon_preprocess_types = True, please?

tk0miya avatar Jul 03 '21 08:07 tk0miya

Thanks for the suggestion. It does now hyperlink, however the hyperlink styling has changed. I'm using sphinx_rtd_theme.

Before: image

After adding napoleon_preprocess_types = True: image

mhostetter avatar Jul 03 '21 13:07 mhostetter

@mhostetter Please open an issue about this on https://github.com/readthedocs/sphinx_rtd_theme/, and if you have a Read the Docs build URL, attach it too so we can easily have a look. I guess napoleon_preprocess_types = True generates different HTML (which is then styled differently).

astrojuanlu avatar Jul 04 '21 21:07 astrojuanlu

I can open a new issue there. I would note, though, that the default theme in sphinx manifests the same error. So I wonder if the issue is in sphinx itself and not necessarily sphinx_rtd_theme.

Without setting napoleon_preprocess_types: image

After setting napoleon_preprocess_types = True: image

When you asked for a "build URL" do you mean a link to the public docs generated from sphinx or a link to the build output from readthedocs website? Let me know how else I can help in the debugging.

mhostetter avatar Jul 05 '21 00:07 mhostetter

Interesting. If the issue is not specific to the theme, then definitely let's continue the conversation here.

I guess this is the function you're talking about: https://galois.readthedocs.io/en/latest/api/galois.trial_division.html#galois.trial_division

The generated HTML is <a class="reference external" href="https://docs.python.org/3/library/functions.html#int" title="(in Python v3.9)"><em>int</em></a>. What happens if napoleon_preprocess_types = True then?

astrojuanlu avatar Jul 06 '21 13:07 astrojuanlu

Without setting napoleon_preprocess_types, the html is

<dt class="field-odd">Parameters</dt>
<dd class="field-odd"><ul class="simple">
<li><p><strong>n</strong> (<a class="reference external" href="https://docs.python.org/3/library/functions.html#int" title="(in Python v3.9)"><em>int</em></a>) – A positive integer.</p></li>
<li><p><strong>B</strong> (<a class="reference external" href="https://docs.python.org/3/library/functions.html#int" title="(in Python v3.9)"><em>int</em></a><em>, </em><em>optional</em>) – The max divisor in the trial division. The default is <code class="samp docutils literal notranslate"><span class="pre">None</span></code> which corresponds to <span class="math notranslate nohighlight">\(B = \sqrt{n}\)</span>.
If <span class="math notranslate nohighlight">\(B &gt; \sqrt{n}\)</span>, the algorithm will only search up to <span class="math notranslate nohighlight">\(\sqrt{n}\)</span>, since a factor of <span class="math notranslate nohighlight">\(n\)</span>
cannot be larger than <span class="math notranslate nohighlight">\(\sqrt{n}\)</span>.</p></li>
</ul>
</dd>
<dt class="field-even">Returns</dt>
<dd class="field-even"><p><ul class="simple">
<li><p><em>list</em> – The discovered prime factors <span class="math notranslate nohighlight">\(\{p_1, \dots, p_k\}\)</span>.</p></li>
<li><p><em>list</em> – The corresponding prime exponents <span class="math notranslate nohighlight">\(\{e_1, \dots, e_k\}\)</span>.</p></li>
<li><p><em>int</em> – The residual factor <span class="math notranslate nohighlight">\(n_r\)</span>.</p></li>
</ul>

and renders like

image

After setting napoleon_preprocess_types = True, the html is

<dt class="field-odd">Parameters</dt>
<dd class="field-odd"><ul class="simple">
<li><p><strong>n</strong> (<a class="reference external" href="https://docs.python.org/3/library/functions.html#int" title="(in Python v3.9)"><code class="xref py py-class docutils literal notranslate"><span class="pre">int</span></code></a>) – A positive integer.</p></li>
<li><p><strong>B</strong> (<a class="reference external" href="https://docs.python.org/3/library/functions.html#int" title="(in Python v3.9)"><code class="xref py py-class docutils literal notranslate"><span class="pre">int</span></code></a>, <em>optional</em>) – The max divisor in the trial division. The default is <code class="samp docutils literal notranslate"><span class="pre">None</span></code> which corresponds to <span class="math notranslate nohighlight">\(B = \sqrt{n}\)</span>.
If <span class="math notranslate nohighlight">\(B &gt; \sqrt{n}\)</span>, the algorithm will only search up to <span class="math notranslate nohighlight">\(\sqrt{n}\)</span>, since a factor of <span class="math notranslate nohighlight">\(n\)</span>
cannot be larger than <span class="math notranslate nohighlight">\(\sqrt{n}\)</span>.</p></li>
</ul>
</dd>
<dt class="field-even">Returns</dt>
<dd class="field-even"><p><ul class="simple">
<li><p><a class="reference external" href="https://docs.python.org/3/library/stdtypes.html#list" title="(in Python v3.9)"><code class="xref py py-class docutils literal notranslate"><span class="pre">list</span></code></a> – The discovered prime factors <span class="math notranslate nohighlight">\(\{p_1, \dots, p_k\}\)</span>.</p></li>
<li><p><a class="reference external" href="https://docs.python.org/3/library/stdtypes.html#list" title="(in Python v3.9)"><code class="xref py py-class docutils literal notranslate"><span class="pre">list</span></code></a> – The corresponding prime exponents <span class="math notranslate nohighlight">\(\{e_1, \dots, e_k\}\)</span>.</p></li>
<li><p><a class="reference external" href="https://docs.python.org/3/library/functions.html#int" title="(in Python v3.9)"><code class="xref py py-class docutils literal notranslate"><span class="pre">int</span></code></a> – The residual factor <span class="math notranslate nohighlight">\(n_r\)</span>.</p></li>
</ul>

and renders like

image

mhostetter avatar Jul 06 '21 18:07 mhostetter

For completeness, I verified the HTML is the same when using the default theme.

mhostetter avatar Jul 07 '21 02:07 mhostetter

Thanks @mhostetter. I think there are two things here:

  1. With the default value of napoleon_preprocess_types, which is False, multiple return types aren't hyperlinked. Given that "when using a single return value, the hyperlinks work as expected", this looks like a bug rather than a feature, but I'm not sure.
  2. With napoleon_preprocess_types = True, extra HTML is being generated. I have no idea whether this makes sense or not.
    • With that extra HTML, I think it's correct that the different themes style it differently.

astrojuanlu avatar Jul 07 '21 08:07 astrojuanlu

FWIW, I agree with your assessment.

mhostetter avatar Jul 07 '21 12:07 mhostetter

Have you guys checked the nested type (e.g., dict[float])?

It seems the hyperlinks for dict and float work correctly in Parameters when napoleon_preprocess_types = False, but will not work at True case. This is for both Parameters and Returns.

It seems impossible to get everything hyperlinked very well. I wish everything could be linked as the case of Parameters with napoleon_preprocess_types = False.

From my understanding, when napoleon_preprocess_types = False, parameter types are translated into :type:. According to the docs, the links are created if possible. But multiple return type won't be translated into :rtype: and there's no link for them.
When napoleon_preprocess_types = True, all types won't get translated into :type: nor :rtype:, but using something like :any: to get the link, which makes the nested types not work.

False

image

True

image

Alternatives

From #9119, @fzalkow proposes an alternative, seems promising:

napoleon_custom_sections = [('Returns', 'params_style')]

But currently, it seems not generating any hyperlink (the same output as False image above).
Is this method not generating :param: and :type:, even though using params_style? That might be a question to the author @SolidifiedRay of PR #8658 .

It seems _parse_custom_params_style_section is calling _format_fields, while _parse_parameters_section is calling _format_docutils_params. Is that the difference? Or it might because of multiple=True or False in _consume_fields?

https://github.com/sphinx-doc/sphinx/blob/4c91c038b220d07bbdfe0c1680af42fe897f342c/sphinx/ext/napoleon/docstring.py#L668-L669

https://github.com/sphinx-doc/sphinx/blob/4c91c038b220d07bbdfe0c1680af42fe897f342c/sphinx/ext/napoleon/docstring.py#L729-L736

ain-soph avatar Oct 28 '21 00:10 ain-soph

To summarise much of the above, napoleon currently translates from something like this:

Returns
-------
list
    The list of values

to this:

:return: The list of values
:rtype: list

But translates from something like this:

Returns
-------
list
    The list of values
None
    No list to return

To this:

:return:
   * *list* – The list of values
   * *None* – No list to return

You can see this decision being made here in the _parse_returns_section().

It's not using the native Sphinx RST format for the return type any more because there isn't a native Sphinx RST format for return type when there's more than one. (This explains why it all works fine if napoleon_preprocess_types = True i.e. when napoleon is manually creating the link anyway. It also explains why there's that small but annoying vertical offset before the first bullet point.)

The key to fixing this issue is making a way to represent multiple return types (and values) natively in RST; then napoleon can be updated to make use of that. I have filed #13920 for exactly that, and posted the easy fix. Once that's done, this is also a fairly easy fix: _parse_returns_section() just needs to change to put the return type in :returns: instead, and do that unconditionally of whether there are multiple types. Something like this (untested!):

    def _parse_returns_section(self, section: str) -> list[str]:
        fields = self._consume_returns_section()
        lines: list[str] = []
        if self._config.napoleon_use_new_return_logic and not any(_name for _name, _type, _desc in fields):
            for _name, _type, _desc in fields:
                field = self._format_field(_name, '', _desc)
                if any(field) or _type:  # only add :returns: if there's something to say
                    lines.extend(self._format_block(':returns ' + _type + ':', field))
        else:
            ...  # Old logic goes here
        if lines and lines[-1]:
            lines.append('')
        return lines

arthur-tacca avatar Nov 06 '25 12:11 arthur-tacca

A couple of secondary notes:

An issue with the above is that it doesn't work with named return values, which were mentioned above. In my opinion, that's fine because return values don't have names. Mainly because this feature should be used for alternative return values (i.e. full return type is Foo | Bar) in which case names certainly don't make sense. Even when used for multiple return types (i.e. full return type is tuple[Foo, Bar]) the names aren't really part of the code, more like suggested names for variables used to destructure it. But anyway, if a fix for that is desired too then that would be a bit trickier but possible. The simplest overall would be to update the underlying Python domain to have a separate directive for named returns using the same field class as :param: (unlike my suggested fix which uses the same field class as :exception:), along with yet another directive for their corresponding types, which just happens to use the same "Returns:" text in the output. But this is all probably a step too far.

As for the note that Astropy doesn't have this issue: I dug through its (complex!) documentation config and found this bottoms out in numpydoc. This doesn't have the issue because it doesn't go through the full translation path (RST autodoc directives -> Google/numpy docstring -> RST Python domain directives -> Python domain code in Sphinx -> docutil nodes). Instead it has its own logic that just directly outputs docutil nodes. If you want multiple return values with correct type links from numpy-style docstrings, then numpydoc might be your best bet.

arthur-tacca avatar Nov 06 '25 12:11 arthur-tacca

Sorry for so much noise but I have realised one more thing: If you want a fix for this right now, today, then you can monkey patch Sphinx to include my suggested changes. That's what I already do for the pure RST part in my little library aioresult (example docstring, example output, using this conf.py). Here's an expanded version of that to include the napoleon part too. I've put it in the fold to avoid adding too much more clutter.

Expand this to see code to monkey patch support for proper return types.

Warning: totally untested (as I don't use napoleon), almost certainly has bugs! Could also break with any new version of Sphinx! Also does not properly support named return values (the names would end up after the type).

# Allow multiple return types to be hyperlinked properly
# https://github.com/orgs/sphinx-doc/discussions/13125#discussioncomment-11219198
# https://github.com/sphinx-doc/sphinx/issues/9394
from sphinx.domains.python import PyObject, PyGroupedField
for i, f in enumerate(PyObject.doc_field_types):
    if f.name == "returnvalue":
        PyObject.doc_field_types[i] = PyGroupedField(
            f.name, label=f.label, names=f.names, rolename="class", can_collapse=True
        )
def _patched_parse_returns_section(self, section: str) -> list[str]:
    fields = self._consume_returns_section()
    lines: list[str] = []
    for _name, _type, _desc in fields:
        field = self._format_field(_name, '', _desc)
        if any(field) or _type:  # only add :returns: if there's something to say
            lines.extend(self._format_block(':returns ' + _type + ':', field))
    if lines and lines[-1]:
        lines.append('')
    return lines
from sphinx.ext.napoleon.docstring import GoogleDocstring
GoogleDocstring._parse_returns_section = _patched_parse_returns_section

arthur-tacca avatar Nov 06 '25 13:11 arthur-tacca