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

Class docstrings are never displayed #19

Closed
francois-rozet opened this issue Apr 17, 2022 · 8 comments
Closed

Class docstrings are never displayed #19

francois-rozet opened this issue Apr 17, 2022 · 8 comments

Comments

@francois-rozet
Copy link

francois-rozet commented Apr 17, 2022

Describe the bug

The class docstrings are never displayed.

Screenshots

With the following class

class SSIM(nn.Module):
    r"""Creates a criterion that measures the SSIM
    between an input and a target.

    Args:
        window_size: The size of the window.
        sigma: The standard deviation of the window.
        n_channels: The number of channels $C$.
        reduction: Specifies the reduction to apply to the output:
            `'none'` | `'mean'` | `'sum'`.
    """

    def __init__(
        self,
        window_size: int = 11,
        sigma: float = 1.5,
        n_channels: int = 3,
        reduction: str = 'mean',
        **kwargs,
    ):
        pass

I get

image

when merge_init_into_class is false and

image

when it is true.

System (please complete the following information):

  • mkdocstrings-python version: 0.6.6
  • Python version: 3.8.10
  • OS: Ubuntu 20.04
plugins:
  - mkdocstrings:
      default_handler: python
      handlers:
        python:
          selection:
            docstring_style: google
          rendering:
            docstring_section_style: list
            heading_level: 3
            members_order: source
            merge_init_into_class: true
            show_bases: false
            show_if_no_docstring: false
            show_root_full_path: true
            show_root_heading: true
            show_source: false
            show_submodules: false
      watch:
        - piqa

Additional context

If I understand correctly the following logic, a docstring is displayed only if merge_init_into_class=true and the __init__ function has a docstring, but it is never the one from the class itself.

{% if config.merge_init_into_class %}
{% if "__init__" in class.members and class.members["__init__"].has_docstring %}
{% with docstring_sections = class.members["__init__"].docstring.parsed %}
{% include "docstring.html" with context %}
{% endwith %}
{% endif %}
{% endif %}

I think that if merge_init_into_class=false or __init__ does not have a docstring, the docstring of the class should be displayed, if any. If merge_init_into_class=true and both __init__ and the class have a docstring, it is more complicated but IMO, the class docstring should have precedence.

@francois-rozet
Copy link
Author

francois-rozet commented Apr 17, 2022

My bad, there is also the following block, but it does not seem to display anything in my case. Any idea why ?

{% with docstring_sections = class.docstring.parsed %}
{% include "docstring.html" with context %}
{% endwith %}

@pawamoy
Copy link
Member

pawamoy commented Apr 17, 2022

You can try to run mkdocs with the -v option to see if there's any warning. You can also verify that Griffe picks up the docstring correctly by running griffe piqa -f -d google | jq .. I see your project is public so I'll try to replicate on my end.

@francois-rozet
Copy link
Author

francois-rozet commented Apr 18, 2022

The griffe command leads to this error:

INFO       Loading package piqa
INFO       Finished loading packages, starting alias resolution
WARNING    piqa/fsim.py:262: No type or annotation for parameter 'chromatic'
Traceback (most recent call last):
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/encoders.py", line 100, in default
    return obj.as_dict(full=self.full, docstring_parser=self.docstring_parser, **self.docstring_options)
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/dataclasses.py", line 173, in as_dict
    base["parsed"] = self.parse(docstring_parser, **kwargs)
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/dataclasses.py", line 154, in parse
    return parse(self, parser or self.parser, **(options or self.parser_options))
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/docstrings/parsers.py", line 51, in parse
    return parsers[parser](docstring, **options)  # type: ignore[operator]
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/docstrings/google.py", line 670, in parse
    section, offset = reader(docstring, offset + 1, **options)  # type: ignore[operator]
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/docstrings/google.py", line 227, in _read_parameters_section
    parameters, new_offset = _read_parameters(docstring, offset)
  File "/home/francois/piqa/lib/python3.8/site-packages/griffe/docstrings/google.py", line 210, in _read_parameters
    if name not in docstring.parent.parameters:  # type: ignore[attr-defined]
AttributeError: 'Class' object has no attribute 'parameters'

If you want to replicate, I have pushed a branch mkdocs in my repo francois-rozet/piqa with the current setup. Happy Easter 🐰

@pawamoy
Copy link
Member

pawamoy commented Apr 18, 2022

Thanks, I'll use that branch to debug! Happy Easter 😄

@pawamoy
Copy link
Member

pawamoy commented Apr 18, 2022

OK I see the issue, its that you define parameters section in docstring classes, and the Google parser tries to access parameters on the class itself (which obviously fails as classes don't have parameters in Griffe's model). I'll make sure the exception is caught to fix it. In the future maybe we can try to use the class' init method instead of the class itself. Note that you can also move the parameters section (args) into the init method, as it's better supported (ability to warn about missing types and unknown parameters, to pick up type annotations in the signature, and to make cross-references of them).

@pawamoy
Copy link
Member

pawamoy commented Apr 18, 2022

Will release soon, closing 🙂

@pawamoy pawamoy closed this as completed Apr 18, 2022
@francois-rozet
Copy link
Author

francois-rozet commented Apr 18, 2022

Thanks for the quick response!

Note that you can also move the parameters section (args) into the init method, as it's better supported (ability to warn about missing types and unknown parameters, to pick up type annotations in the signature, and to make cross-references of them).

It does work, but it has the drawback of splitting the docstring in two and forcing the parameters section below everything else. Since it is very common to document the parameters in the class docstring, it would be best, as you said, to use the __init__ parameters as the class parameters.

For instance, adding to the Class class in griffe, a property parameters?

@property
def parameters(self):
    return self.members['__init__'].parameters

Edit: just checked, and it works very well!

@pawamoy
Copy link
Member

pawamoy commented Apr 18, 2022

Ah, clever! Do you want to send a PR 😄?

pawamoy pushed a commit to mkdocstrings/griffe that referenced this issue Apr 18, 2022
Issue mkdocstrings/python#19: mkdocstrings/python#19
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
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

2 participants