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

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

Closed
cpcloud opened this issue Apr 8, 2023 · 7 comments
Closed

Comments

@cpcloud
Copy link

cpcloud commented Apr 8, 2023

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):

  • griffe version: [e.g. 0.2.1]
  • Python version: 3.10
  • OS: Linux
@pawamoy
Copy link
Member

pawamoy commented Apr 8, 2023

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
Copy link
Author

cpcloud commented Apr 8, 2023

@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
Copy link
Member

pawamoy commented Apr 8, 2023

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
Copy link
Member

pawamoy commented Apr 8, 2023

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

@pawamoy
Copy link
Member

pawamoy commented Apr 8, 2023

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
Copy link
Author

cpcloud commented Apr 8, 2023

Neat, that works!

image

Thank you so much!

@pawamoy
Copy link
Member

pawamoy commented Apr 8, 2023

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

@pawamoy pawamoy closed this as completed Apr 8, 2023
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