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

Return value section loses text in backticks #96

Closed
wch opened this issue Apr 12, 2023 · 9 comments
Closed

Return value section loses text in backticks #96

wch opened this issue Apr 12, 2023 · 9 comments
Labels
.question Further information is requested shiny type:documentation Improvements or additions to documentation

Comments

@wch
Copy link
Collaborator

wch commented Apr 12, 2023

For example, the input text in the docstring is this (https://github.com/rstudio/py-shiny/blob/8cafc4470691961a8f994bcfc819d13d19f9deed/shiny/ui/_notification.py#L54-L56):

    Returns
    -------
    The notification's ``id``.

However, the output qmd looks like this:


## Returns

| Type   | Description        |
|--------|--------------------|
| The notification's  | |

And the HTML:

image

I tried changing the double backticks around id to single backticks, but it made no difference.

@machow
Copy link
Owner

machow commented Apr 12, 2023

I don't think the returns section conforms to numpydoc style. It looks like the parser thinks the description is type information. Do you know if that worked with sphinx?

numpydoc has two styles for return sections:

Returns
-------
int
    Description of anonymous integer return value.
Returns
-------
err_code : int
    Non-zero value indicates error code, or zero on success.
err_msg : str or None
    Human readable error message, or None on success.

https://numpydoc.readthedocs.io/en/latest/format.html#returns

@machow machow added the .question Further information is requested label Apr 12, 2023
@wch
Copy link
Collaborator Author

wch commented Apr 13, 2023

In all of our code, the functions have a return type annotation, like this:

def f(x: int) -> str:
    """
    <docstring text here>
    
    Returns
    --------
    A string representation of `x`.
    """
     ...

Can quartodoc use that return type information?

Perhaps the resulting markdown could look something like this:

## Returns

**str**: A string representation of `x`.


With the numpydoc's second style of return documentation, does that mean the returned value is a tuple? In practice, I think that is very uncommon, at least in our code. For us, if we return a tuple, I think it makes sense to document that the return value is a tuple with specified types, instead of using a table.

So if we had a function like this:

def g(x: int) -> tuple(int, str | None):
    """
    <docstring text here>

    Returns
    --------
    A tuple where the first value is a return code, and the second value is an error message, or `None` if no error.
    """
     ...

The resulting markdown could be like this:

## Returns

**tuple(int, str | None)**: A tuple where the first value is a return code, and the second value is an error message, or `None` if no error.

@machow
Copy link
Owner

machow commented Apr 13, 2023

quartodoc uses numpydoc style, so if numpydoc supports it, then we should support it too.

For the example you gave, it seems like you are using what numpydoc parses as the type field.

from numpydoc.docscrape import NumpyDocString
ds = NumpyDocString(
    """
    <docstring text here>

    Returns
    --------
    A tuple where the first value is a return code, and the second value is an error message, or `None` if no error.
    """
)

ds["Returns"]
[Parameter(name='', type='A tuple where the first value is a return code, and the second value is an error message, or `None` if no error.', desc=[])]

It's surprising numpydoc all return styles involve writing out the type. I want to be careful not to deviate from numpydoc style, but I think you should be able to override the render of DocstringSectionReturns

@wch
Copy link
Collaborator Author

wch commented Apr 13, 2023

From #90 (but that comment was really about this issue):

Why do you want backticks in the type column? I don't think there is a way to have a valid type annotation with backticks

I don't want backticks in the type column. I actually don't want a table, but even when I provided a customized _render function that puts the text in the second column, the code still disappears.

For example, see https://shiny.rstudio.com/py/api/ui.notification_show.html. This should say "The notification's id", but the id is not there.

image

@machow machow added this to quartodoc Apr 14, 2023
@machow
Copy link
Owner

machow commented Apr 14, 2023

tl;dr does this syntax work for you?

Using this syntax seems to work with the griffe parser (cf mkdocstrings/griffe#137; please raise an issue there if you are concerned about stability of the behavior):

"""
Returns
--------
:
    Some description
"""

Example

file: test_func.py

def f(x) -> int:
    """
    Returns
    -------
    :
        the description
    """
preview(get_object("test_func.f").docstring)
█─Docstring
├─parser = <Parser.numpy: 'numpy'>
└─parsed = █─list
           └─0 = █─DocstringSectionReturns
                 ├─kind = <DocstringSectionKind.returns: 'returns'>
                 ├─title = None
                 └─value = █─list
                           └─0 = █─DocstringReturn      # NOTE THIS RETURNS SECTION -----------------
                                 ├─name = ''
                                 ├─annotation = Name(source='int', full='int')
                                 └─description = 'the description'

Background

It sounds like there are two issues at play:

  1. numpydoc style: you don't want to have to specify type in the returns section (since the return annotation specifies it)
  2. griffe parser: things in backticks in the return type annotation get dropped. seems less like a problem.

It looks like griffe handles automatically inferring the return type, when you just use "some_name: " in returns with no type on the right-hand side.

from griffe.dataclasses import Docstring
from griffe.docstrings.parsers import parse_numpy
from quartodoc import preview

doc = Docstring("""
    Returns
    -------
    :
        some `backtick` thing
    """
)
preview(parse_numpy(doc))
█─list
└─0 = █─DocstringSectionReturns
      ├─kind = <DocstringSectionKind.returns: 'returns'>
      ├─title = None
      └─value = █─list
                └─0 = █─DocstringReturn
                      ├─name = ''
                      ├─annotation = 'a'
                      └─description = 'some `backtick` thing'

This is not what numpydoc does (since it only uses the docstring), but it makes sense to me that griffe might do some more nuanced handling here, since it does have return type information:

from numpydoc.docscrape import NumpyDocString
ds = NumpyDocString(
    """
    <docstring text here>

    Returns
    --------
    :
        A tuple where the first value is a return code, and the second value is an error message, or `None` if no error.
    """
)

ds["Returns"]
[Parameter(name='', type=':', desc=['A tuple where the first value is a return code, and the second value is an error message, or `None` if no error.'])]

@machow machow added the type:documentation Improvements or additions to documentation label Apr 14, 2023
@machow machow moved this to Review in quartodoc Apr 14, 2023
@jcheng5
Copy link

jcheng5 commented Apr 18, 2023

@machow I just noticed that the Return value here is truncated: https://shiny.rstudio.com/py/api/render.plot.html (compare to the docstring

Should we be going through the py-shiny repo and changing all the docstrings to have the dummy : for the Returns section?

@machow machow added the shiny label Apr 18, 2023
@machow
Copy link
Owner

machow commented Apr 18, 2023

Yeah -- writing the docstrings like this should fix it, and pull in the correct return type:

def f(x) -> int:
    """
    Returns
    -------
    :
        the description
    """

@wch
Copy link
Collaborator Author

wch commented Apr 18, 2023

Update: I've updated the docstrings in shiny to use : as the return type.

@machow
Copy link
Owner

machow commented May 2, 2023

@machow machow closed this as completed May 2, 2023
@github-project-automation github-project-automation bot moved this from Review to Done in quartodoc May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.question Further information is requested shiny type:documentation Improvements or additions to documentation
Projects
Status: Done
Development

No branches or pull requests

3 participants