mkdocstrings / python

A Python handler for mkdocstrings.
https://mkdocstrings.github.io/python
ISC License
175 stars 32 forks source link

bug: mkdocstrings not collecting type information from parent classes #175

Open coltonbh opened 1 month ago

coltonbh commented 1 month ago

Description of the bug

mkdocstrings does not collect type information from parent classes.

To Reproduce

inherited_members: true

 Example:

   from pydantic import BaseModel

    class First(BaseModel):
        """First class

        Attributes:
            val: A string value.
        """
        val: str

    class Second(First):
        """Second Class

        Attributes:
            val: A string value.
            val2: An integer value.
        """
        val2: int

The val1 type information is missing on the documentation:

image

Expected behavior

I would expect mkdocstrings to get the type information from the parent class and display it.

If I omit the value from the docstring then it is hidden. This makes me think the intention is to hide inherited values? Again, this can't be right because I'd want concrete classes to display mixin values they have inherited. I've been over the docs extensively but can't seem to find a setting that makes this work.

from pydantic import BaseModel

class First(BaseModel):
    """First class

    Attributes:
        val: A string value.
    """

    val: str

class Second(First):
    """Second Class

    Attributes:
        val2: An integer value.
    """

    val2: int

image

Environment information

Is this expected behavior? In general I'd like to be able to pull type information from mixin classes directly rather than having to add type annotations to the docstrings. I must be doing something wrong! Thanks for your help :)

pawamoy commented 1 month ago

Hi @coltonbh, thanks for the report! I'll mark this as a bug. Currently, when parsing docstrings, we don't check inherited members to obtain the type. I suppose we could do that. You did nothing wrong, it's just the first time I see this use-case and the first time it is reported :slightly_smiling_face:

coltonbh commented 1 month ago

Thanks, @pawamoy! I'll extend this just a bit further. Often I have inheritance hierarchies where a base class has attributes shared by all children, and then children are composed of various mixins and their own attributes like this:

from typing import Any, Dict, List

from pydantic import BaseModel

class BaseClass(BaseModel):
    """Base class

    Attributes:
        extras: Dictionary for extra values.
    """

    extras: Dict[str, Any] = {}

class _Mixin1(BaseClass):
    """Mixin 1 class

    Attributes:
        val1: A string value.
    """

    val1: str

class _Mixin2(BaseClass):
    """Mixin 2 class

    Attributes:
        val2: An integer value.
    """

    val2: int

class First(BaseClass, _Mixin1):
    """First class

    Attributes:
        first: A float value.
    """

    first: float

class Second(BaseClass, _Mixin2):
    """Second Class

    Attributes:
        second: A list of integers.
    """

    second: List[int]

Ideally, I'd like to be able to just declare the type information once (on the parent object) and then have the inherited attributes and their docstrings be included in children classes. So ideally, the code above would produce an Attributes table for First with extras, val1, and first. Instead, if I want First to have correct documentation I have to do the following, which when I have many children classes requires duplicating tons of docstring information:

from typing import Any, Dict, List

from pydantic import BaseModel

class BaseClass(BaseModel):
    """Base class

    Attributes:
        extras: Dictionary for extra values.
    """

    extras: Dict[str, Any] = {}

class _Mixin1(BaseClass):
    """Mixin 1 class

    Attributes:
        val1: A string value.
    """

    val1: str

class _Mixin2(BaseClass):
    """Mixin 2 class

    Attributes:
        val2: An integer value.
    """

    val2: int

class First(BaseClass, _Mixin1):
    """First class

    Attributes:
        first: A float value.
        val1 (str): A string value.
        extras (Dict[str, Any]): Dictionary for extra values.
    """

    first: float

class Second(BaseClass, _Mixin2):
    """Second Class

    Attributes:
        second: A list of integers.
        val2 (int): An integer value.
        extras (Dict[str, Any]): Dictionary for extra values.
    """

    second: List[int]

I have to keep repeating the declaration for extras on all children classes and duplicating the docstring and adding type information. This holds for all children classes so I may end up with the extras (Dict[str, Any]): Dictionary for extra values. docstring duplicated 10-15x throughout my code base.

Ideally I'd love for the types and docstrings to come with inherited classes so I only have to document an attribute once and it will flow through to all children :)

As a small follow-on question, is it possible to have @property attributes show up in the Attributes table without explicitly including them in the docstring? E.g., I'd want computed_attr to be in the Attributes table without having to duplicate its docstring into the class docstring?

class BaseClass(BaseModel):
    """Base class

    Attributes:
        extras: Dictionary for extra values.
    """

    extras: Dict[str, Any] = {}

    @property
    def computed_val(self) -> int:
        """A computed value"""
        return len(self.extras)

image

pawamoy commented 1 month ago

Ideally, I'd like to be able to just declare the type information once (on the parent object) and then have the inherited attributes and their docstrings be included in children classes.

That's already the case, but not for attributes sections as you would expect, see next paragraph.

So ideally, the code above would produce an Attributes table for First with extras, val1, and first.

That won't happen. Docstring sections will by default remain explicit only, always. Griffe itself will never implicitly modify docstring sections: if you document a single item, it won't add other items for you. But this can be achieved with an extension if you really want it! However, consider using actual attribute docstrings, which we recommend instead of Attributes section:

from typing import Any, Dict, List

from pydantic import BaseModel

class BaseClass(BaseModel):
    """Base class."""

    extras: Dict[str, Any] = {}
    """Dictionary for extra values."""

class _Mixin1(BaseClass):
    """Mixin 1 class."""

    val1: str
    """A string value."""

class _Mixin2(BaseClass):
    """Mixin 2 class."""

    val2: int
    """An integer value."""

class First(BaseClass, _Mixin1):
    """First class."""

    first: float
    """A float value."""

class Second(BaseClass, _Mixin2):
    """Second Class."""

    second: List[int]
    """A list of integers."""

The benefit of documenting each attribute instead of writing Attributes sections in parent classes is that each attribute gets a permalink and an entry in the objects inventory generated by mkdocstrings, which means it can be cross-referenced.

As a small follow-on question, is it possible to have @property attributes show up in the Attributes table without explicitly including them in the docstring? E.g., I'd want computed_attr to be in the Attributes table without having to duplicate its docstring into the class docstring?

If you document each attribute and property, mkdocstrings can auto-generate summary tables (attributes, functions, classes, etc.) for you. That's an insiders feature though: https://mkdocstrings.github.io/python/usage/configuration/members/#summary.

If you want to customize your attributes sections anyway, as said before you can write a Griffe extension :slightly_smiling_face: See https://mkdocstrings.github.io/griffe/guide/users/extending/.

Let me know what you think.

coltonbh commented 1 month ago

Ah! Thank you for the details on this! Did not know about the attribute docstrings. I'm headed out of town for the next week but will play with this when I get back. I'm about to make documentation for a whole suite of packages so I'm keen to get the best practices in place and then roll them through my whole stack :) Thank you!

pawamoy commented 1 month ago

You're welcome! Over a few years I've accumulated some issues that I labeled "docs". This will be useful when I finally have some time to write a proper mkdocstring-python guide for writing docstrings :slightly_smiling_face:

coltonbh commented 1 month ago

I would love a "best practices guide!" Just a straightforward "here's what we recommend and why," something that works for 90% of the cases. The flexibility of mkdocstrings is impressive--it's also easy to spend hours looking through all the options to see if it does what you have in mind and then wondering if you're crazy (in this case because I'm duplicating information) or if I'm just doing it wrong or if I'm missing some concept that would explain why what I'm doing isn't best practice :) A simple "how-to" would be awesome :)

coltonbh commented 1 month ago

Ok back online...

Really appreciate the detailed response.

Two final questions

  1. Even if I go with your suggested approach:
    
    from typing import Any, Dict, List

from pydantic import BaseModel

class BaseClass(BaseModel): """Base class."""

extras: Dict[str, Any] = {}
"""Dictionary for extra values."""

class _Mixin1(BaseClass): """Mixin 1 class."""

val1: str
"""A string value."""

class _Mixin2(BaseClass): """Mixin 2 class."""

val2: int
"""An integer value."""

class First(BaseClass, _Mixin1): """First class."""

first: float
"""A float value."""

class Second(BaseClass, _Mixin2): """Second Class."""

second: List[int]
"""A list of integers."""

The concrete classes `First` and `Second` are missing documentation for their inherited attributes, like `extras` and `val1`/`val2`. 

![image](https://github.com/user-attachments/assets/8a1b1718-bed0-4661-ade4-d50acd390759)

Is there a way you recommend for making the documentation show inherited attributes? Currently the google docstring approach seems best to me since I get a nice, explicit attributes table; however, I'm open to the attribute docstrings approach you suggest, especially if I could bring through inherited attributes nicely.

2. What's the reason for treating "attribute docstrings" differently from writing this same content inside the docstring of the function/class with google docstrings? I think there's some distinction between these two approaches I'm missing. Naively I'd think they are both the exact same thing--a string describing an attribute--so they'd be treated the same inside `mkdocs` representation used to build the docs. But you highlight this advantage, so I must be missing something:

> The benefit of documenting each attribute instead of writing Attributes sections in parent classes is that each attribute gets a permalink and an entry in the objects inventory generated by mkdocstrings, which means it can be cross-referenced.
pawamoy commented 1 month ago

Is there a way you recommend for making the documentation show inherited attributes?

Yes, see https://mkdocstrings.github.io/python/usage/configuration/members/#inherited_members.

What's the reason for treating "attribute docstrings" differently from writing this same content inside the docstring of the function/class with google docstrings?

Good question. Griffe makes a distinction between an Attributes docstring section, and the set of documented attributes (with attribute docstrings, written below their assignment), because you can have both in your code:

class Hello:
    """Hello.

    Attributes:
        hello: Hello.
    """

    hello: str = "Hello"
    """Hello."""

Then mkdocstrings-python treats sections as simple summary tables (or lists, depending on docstring_section_style), while documented attributes are given a proper heading (and permalink, and toc entry, and objects inventory entry).

We have a feature request in our backlog to generate headings for items in docstring sections too, see https://github.com/mkdocstrings/python/issues/133.

coltonbh commented 1 month ago

Thanks for all the details! I think I'm ready to take on some serious documentation work now ;)