False blank line and unindent warnings

[01-Jacky]

I have a project using sphinx 2.2.2, sphinx.ext.autodoc, and latest sphinx_autodoc_typehints

Using these two extensions and the below flags in conf.py causes "WARNING: Field list ends without a blank line; unexpected unindent." in the __init__ function of a class.

autoclass_content = "both"
always_document_param_types = True

The docstrings to be appears fine on this demo class

class Foo:
    """Class to demo a bug"""

    def __init__(self, magic_num: int) -> None:
        """Creates the Foo

        :param magic_num: some magic number

        """
        self.magic_num = magic_num

Looks like the html output ends up generating parameters twice, might relate to that?

This seems run fine if I switch to sphinx_autodoc_typehints 1.7.0 but anything above causes this warning.

Here's a demo repo that reproduce the bug. https://github.com/01-Jacky/autodoc-typehints-bug

[gaborbernat]

@agronholm so ran into this just now, https://github.com/agronholm/sphinx-autodoc-typehints/blob/master/tests/roots/test-dummy/dummy_module.py#L24-L26 shows the parameters being defined at class level instead at constructor? Is this expected/supported? I personally never seen this, and I guess the generation also expects this. PyCharm definitely wants you to define argument documentation at init level, and not at class level. At class level you should be using var/ivar at the very least 🤔

At the very least we should support both formats IMHO. Do you agree?

[gaborbernat]

Quoting https://stackoverflow.com/a/62004004/296075:

The class documentation should include the public components of the object. The __init__ parameters may or may not be public, so whether they are included in the class docstring or not depends on the object design.

The full paragraph in PEP 257 states:

The docstring for a class should summarize its behavior and list the
public methods and instance variables. If the class is intended to be
subclassed, and has an additional interface for subclasses, this
interface should be listed separately (in the docstring). The class
constructor should be documented in the docstring for its __init__
method. Individual methods should be documented by their own
docstring.

And the google style guide states:

Classes should have a docstring below the class definition describing
the class. If your class has public attributes, they should be
documented here in an Attributes section and follow the same
formatting as a function’s Args section.

So the documentation determination hinges on whether the parameters for __init__ are public. If the intention of the object is for users to construct their own instances, the __init__ parameters should be documented in the class docstring. However, if an object is constructed internally then returned to the user, the class documentation should only refer to the public aspects of the object.

So the following example from google suggest that the likes_spam parameter from __init__ is public:

class SampleClass(object):
    """Summary of class here.

    Longer class information....
    Longer class information....

    Attributes:
        likes_spam: A boolean indicating if we like SPAM or not.
        eggs: An integer count of the eggs we have laid.
    """

    def __init__(self, likes_spam=False):
        """Inits SampleClass with blah."""
        self.likes_spam = likes_spam
        self.eggs = 0

    def public_method(self):
        """Performs operation blah."""

However, below the public likes_spam attribute is determined during __init__ based on two internal parameters spam_count and like_threshold. So, likes_spam is documented in the class docstring, while spam_count and like_threshold are documented in the __init__ docstring.

class SampleClass(object):
    """Summary of class here.

    Longer class information....
    Longer class information....

    Attributes:
        likes_spam: A boolean indicating if we like SPAM or not.
        eggs: An integer count of the eggs we have laid.
    """

    def __init__(self, spam_count, like_threshold=1):
        """Inits SampleClass.

        Args:
            spam_count: The amount of SPAM consumed.
            like_threshold: The threshold consumed that indicates 
                whether we like SPAM.
        """
        self.likes_spam = spam_count > like_threshold
        self.eggs = 0

    def public_method(self):
        """Performs operation blah."""

So seems documenting parameters at class level implies they are public attributes on the class. So we should probably better handle things here and not merge __init__ docstring and the class level docstring.

[gaborbernat]

Consider the following example that uses the two separately:

class InitWithPrivateAndPublic:
    """
    This is a great class.

    :param pub_1: yes it's public
    :param pub_2: this too
    """

    def __init__(self, pub_1: str, magical: int, priv_1: float) -> None:
        """
        Class constructor.

        :param pub_1: pass through
        :param magical: this is const only
        :param priv_1: this is private kept
        """
        self.pub_1 = pub_1
        self.pub_2 = magical + 1
        self._priv_1 = priv_1

I'd expact both the class and the constructor level arguments to be annotated separately.

[agronholm]

That example makes zero sense to me. Why would you have parameter declarations in both the class docstring and in __init__()?

[gaborbernat]

Because one documents arguments to the constructor while the other is documenting class attributes. They're for different scope/purpose.

[agronholm]

You were using :param in both though. That's what didn't make any sense to me. If you're documenting attributes, you should use .. attribute:: or :ivar:.
How many projects have you seen where __init__() is separately documented from the class? I can't think of any off the top of my head. The Python standard library itself documents the parameters in the class docstring too.

[gaborbernat]

You were using :param in both though. That's what didn't make any sense to me. If you're documenting attributes, you should use .. attribute:: or :ivar:.

At very least though this tool should not try to inject type annotations for the class if the constructor has param.

d. The Python standard library itself documents the parameters in the class docstring too.

Not sure why it does so, feels to go against PEP-257:

The docstring for a class should summarize its behavior and list the
public methods and instance variables. If the class is intended to be
subclassed, and has an additional interface for subclasses, this
interface should be listed separately (in the docstring). The class
constructor should be documented in the docstring for its init
method. Individual methods should be documented by their own
docstring.