mkdocstrings / griffe

Signatures for entire Python programs. Extract the structure, the frame, the skeleton of your project, to generate API documentation or find breaking changes in your API.
https://mkdocstrings.github.io/griffe
ISC License
320 stars 44 forks source link

Griffe appears to ignore the effect of decorators that modify docstrings #142

Closed cpcloud closed 1 year ago

cpcloud commented 1 year ago

Describe the bug

Docstrings in functions that have docstrings modified by a decorator are lost.

To Reproduce Steps to reproduce the behavior:

In ibis we have [an @experimental decorator]() that modifies __doc__ to include an admonition warning users with a generic message about APIs being subject to change.

This used to show up before mkdocstrings switched to griffe, but now it doesn't 😄.

Expected behavior

I would expect the docstring's full content to be included when rendering.

~Screenshots~ How to See the Problem

Here's a couple ways to see it

  1. The rendered ibis docs for pivot_wider
  2. Here's a Python session showing that Griffe has the original docstring, but Python has the computed one:

Griffe's View

>>> from griffe.loader import GriffeLoader
>>> loader = GriffeLoader()
>>> module = loader.load_module("ibis.expr.types.relations")
>>> ds = module.classes["Table"].functions["pivot_wider"].docstring
>>> print('\n'.join(ds.lines[:30]))
Pivot a table to a wider format.

Parameters
----------
id_cols
    A set of columns that uniquely identify each observation.
names_from
    An argument describing which column or columns to use to get the
    name of the output columns.
names_prefix
    String added to the start of every column name.
names_sep
    If `names_from` or `values_from` contains multiple columns, this
    argument will be used to join their values together into a single
    string to use as a column name.
names_sort
    If [`True`][True] columns are sorted. If [`False`][False] column
    names are ordered by appearance.
names
    An explicit sequence of values to look for in columns matching
    `names_from`.

    * When this value is `None`, the values will be computed from
      `names_from`.
    * When this value is not `None`, each element's length must match
      the length of `names_from`.

    See examples below for more detail.
values_from
    An argument describing which column or columns to get the cell

Python's View via __doc__

Pivot a table to a wider format.

        !!! warning "This API is experimental and subject to change."

        Parameters
        ----------
        id_cols
            A set of columns that uniquely identify each observation.
        names_from
            An argument describing which column or columns to use to get the
            name of the output columns.
        names_prefix
            String added to the start of every column name.
        names_sep
            If `names_from` or `values_from` contains multiple columns, this
            argument will be used to join their values together into a single
            string to use as a column name.
        names_sort
            If [`True`][True] columns are sorted. If [`False`][False] column
            names are ordered by appearance.
        names
            An explicit sequence of values to look for in columns matching
            `names_from`.

            * When this value is `None`, the values will be computed from
              `names_from`.
            * When this value is not `None`, each element's length must match
              the length of `names_from`.

            See examples below for more detail.

The differences in indentation aren't interesting, but this line is:

!!! warning "This API is experimental and subject to change."

System (please complete the following information):

pawamoy commented 1 year ago

Hey @cpcloud!

This is expected: Griffe by default does not execute source code (contrary to pytkdocs), it parses it and visits the AST. Griffe is still able to inspect code (like pytkdocs). I've been working on a way to selectively inspect objects with the hybrid built-in extension. I'll show you an example a bit later!

Additionally/alternatively, maybe you'd prefer to tell Griffe to inspect rather than visit your whole package, with a config option? I'd not recommend it though, as visiting code is required for handling stubs, type-guarded things, and ephemeral data like overloaded signatures.

cpcloud commented 1 year ago

@pawamoy 👋🏻 Hey!

Selective inspection sounds like a good option for this for us.

Are inspection and visitation mutually exclusive?

For example, if I tell griffe to inspect pivot_wider does that mean I'll lose the type information in the rendered docs?

pawamoy commented 1 year ago

Great!

No they're not mutually exclusive: it's absolutely possible to both visit and inspect an object, merging data in a way that makes sense. Currently it's actually the user's responsibility to say how to merge the data together. Maybe in the future we'll have some smart algorithm to merge visited and inspected data.

By the way, it might not even be necessary to inspect your decorated functions: we could use a visitor extension that simply detects when the decorator is used, and that updates the docstring accordingly. I'll share something later :)

pawamoy commented 1 year ago

Could you fix your link to the experimental decorator? I'd like to see how you use it.

pawamoy commented 1 year ago

Nevermind, found what I needed.

So, here's an extension example that updates docstrings when it finds a function decorated with experimental:

# experimental_admonition.py
from __future__ import annotations

import ast

from griffe.dataclasses import Function
from griffe.extensions import VisitorExtension

from ibis.utils import append_admonition

class ExperimentalAdmonitionExtension(VisitorExtension):
    def visit_functiondef(self, node: ast.FunctionDef) -> None:
        function: Function = self.visitor.current.members[node.name]  # type: ignore[assignment]
        for decorator in function.decorators:
            if decorator.value == "experimental" or decorator.value.endswith(".experimental"):
                function.docstring.value = append_admonition(
                    function.docstring.value,
                    msg="This API is experimental and subject to change.",
                )

Extension = ExperimentalAdmonitionExtension

You'd have to refactor append_admonition so that it takes a string and returns an updated string, rather than taking a callable directly (which does not exist when visiting code).

Now you can load this extension from mkdocs.yml with:

plugins:
- mkdocstrings:
    handlers:
      python:
        options:
          extensions:
          - experimental_admonition.py
cpcloud commented 1 year ago

Neat, that works!

image

Thank you so much!

pawamoy commented 1 year ago

Great! I'll close this, this will be added to the docs soon :slightly_smiling_face: