Explicit explanation how to write docstring

[keisuke-umezawa]

As contributors write docstrings for their implementations, it is better to show an explicit example how to write docstrings.
e.g. in pytorch reference, it refers Google style docstrings.

Currently, in optuna, there are no explicit explanation for docstrings in the coding guideliens. Just referring pep8, and it is difficult to know how docstrings should be.

[arpitkh101]

Same problem

[hvy]

@keisuke-umezawa thanks for bringing this issue up. Do you have any ideas for how to address it?

[keisuke-umezawa]

@hvy
For docstring guidelines, basically I would like to follow Example Google Style Python Docstrings.

But there are several differences between it and what we use. So it is better to write our docstrings examples?

def function_with_pep484_type_annotations(param1: int, param2: str) -> bool:
    """Example function with PEP 484 type annotations.
    
    Example:
        Using `testsetup` and `testcode`.

        .. testsetup::
            import numpy as np
                
        .. testcode::
            x = np.zeros(10)

    Args:
        param1:
            The first parameter.
        param2:
            The second parameter.

    Returns:
        The return value. True for success, False otherwise.

    """
    return True

class ExampleClass(object):
    """The summary line for a class docstring should fit on one line.

    If the class has public attributes, they may be documented here
    in an ``Attributes`` section and follow the same formatting as a
    function's ``Args`` section.

    Properties created with the ``@property`` decorator should be documented
    in the property's getter method.

    Attributes:
        attr1:
            Description of `attr1`.
        attr2:
            Description of `attr2`.

    """

    def __init__(self, param1: str, param2: Optional[int] = 0):
        """Example of docstring on the __init__ method.

        `__init__` method must be documented in `__init__` method itself,
        not the class level docstring.

        Note:
            Do not include the `self` parameter in the ``Args`` section.

        Args:
            param1:
                Description of `param1`.
            param2:
                Description of `param2`. Multiple
                lines are supported.

        """
        self.attr1 = param1
        self.attr2 = param2


    @property
    def readonly_property(self) -> str:
        """Properties should be documented in their getter method."""
        return 'readonly_property'

    @property
    def readwrite_property(self) -> List[str]:
        """Properties with both a getter and setter
        should only be documented in their getter method.

        If the setter method contains notable behavior, it should be
        mentioned here.
        """
        return ['readwrite_property']

    @readwrite_property.setter
    def readwrite_property(self, value: int) -> int:
        value

    def example_method(self, param1: str, param2: int) -> bool:
        """Class methods are similar to regular functions.

        Example:
            Using `testsetup` and `testcode`.

            .. testsetup::
                import numpy as np
                
            .. testcode::
                x = np.zeros(10)

        Note:
            Do not include the `self` parameter in the ``Args`` section.

        Args:
            param1:
                The first parameter.
            param2:
                The second parameter.

        Returns:
            True if successful, False otherwise.

        """
        return True

Main differences from Example Google Style Python Docstrings

• Add Example: sections.
• Args and Attributes sections always starts with a new line.
• No inline docstrings.
• __init__ method must be documented in __init__ method itself, not the class level docstring.

I want to add above into the coding guideliens

[keisuke-umezawa]

One question is that:
Which is better way to test output? e.g. https://github.com/optuna/optuna/pull/882/files#diff-815bd38671f30e415efa796835df638fR771

1. Ussing assert to test output in directive-testcode
2. Ussing directive-testoutput to test output

[hvy]

#933 (comment)

I basically agree with the above comment and that it'd improve readability. I'd like to hear opinions from other developers. My concern is how to maintain the docstrings, especially if we start customizing the rules. Can we use black for that, or yapf (c.f. #956)?

[hvy]

#933 (comment) (latter question)

I personally find prints to be verbose and rather uninformative in testcode. I this either 1., or a standard comment is fine, as these doctests are not unit tests but merely there to make sure they are up to date.

[keisuke-umezawa]

Can we use black for that, or yapf (c.f. #956)?

In my understanding, formatters do not change comments, so we can use black or yapf with any customized docstring guidelines.

[keisuke-umezawa]

But, I also do not like customized guidelines, and as much as possible I want to follow Example Google Style Python Docstrings.

• Add Example: sections.
• Args and Attributes sections always starts with a new line.
• No inline docstrings.
• init method must be documented in init method itself, not the class level docstring.

For those, we can ignore the differences above. It means we can do following:

• When you want to add Add Example: sections, you can do it.
• Args and Attributes sections can start with a new line or without a new line.
• OK to use inline docstring.
• __init__ method can be documented in __init__ method itself, or the class level docstring.

And, in the coding guideliens, we can just add following

[keisuke-umezawa]

• We follow the docstring style defined by Example Google Style Python Docstrings.

Optuna Specific Conventions

Add examples with doctest

If you want to add an example, you can add an Example section in docstring.
It should be tested by doctest e.g.

def function_with_pep484_type_annotations(param1: int, param2: str) -> bool:
    """Example function with PEP 484 type annotations.
    
    Example:
        Using `testsetup` and `testcode`.

        .. testsetup::
            import numpy as np
                
        .. testcode::
            x = np.zeros(10)

    Args:
        param1:
            The first parameter.
        param2:
            The second parameter.

    Returns:
        The return value. True for success, False otherwise.

    """
    return True

[toshihikoyanase]

@keisuke-umezawa Thank you for your proposal. It basically sounds good to me.
I found some differences with the current convention such as

• an empty line after function definition (c.f., https://github.com/optuna/optuna/wiki/Coding-Style-Conventions#an-empty-line-after-a-functionclass-definition), and
• Sphinx-style link to Python objects like True and False (e.g., https://github.com/optuna/optuna/blob/v1.2.0/optuna/trial.py#L540).

If we follow the current coding convention, the example will be as follows:

def function_with_pep484_type_annotations(param1: int, param2: str) -> bool:
    """Example function with PEP 484 type annotations.
    
    Example:
        Using `testsetup` and `testcode`.

        .. testsetup::
            import numpy as np
                
        .. testcode::
            x = np.zeros(10)

    Args:
        param1:
            The first parameter.
        param2:
            The second parameter.

    Returns:
        The return value. :obj:`True` for success, :obj:`False` otherwise.

    """

    return True

I think we need to choose to keep the current convention.
The first one (empty lines) can be removed from the rules if it cannot be corrected using tools automatically. As for the second one, I think it is useful for users not mandatory because it seems to be a common sense of Python users.
What do you think of it?

[hvy]

+1 for omitting the "empty line" rule for reasons already mentioned. It's not something we want to nitpick on during reviews.

[keisuke-umezawa]

Thank you for the comments, and sorry for late relply. I want to add following changes into the coding guideliens:

1. Delete the rule for an empty line after function definition. https://github.com/optuna/optuna/wiki/Coding-Style-Conventions/_compare/d4b24e62312bbf8669e94dc36efdc0d0ffb13053...70e1e5425b3ceb5fbf9f67f77617ebdcf6afc8dc
2. Add docstrings guidelines. https://github.com/optuna/optuna/wiki/Coding-Style-Conventions/_compare/70e1e5425b3ceb5fbf9f67f77617ebdcf6afc8dc...f6f8e95483870937e9a7a28cb24a26e3d4d0d817

[hvy]

Thanks for the updates and the separate diffs (those help). I'd like to verify this with other core developers and committers but changes basically LGTM. I have two questions.

• Are we going to progressively update our documentation (e.g. contribution-welcome) to match these rules?
• Does Note: do the same thing as .. note:: ?

[keisuke-umezawa]

Are we going to progressively update our documentation (e.g. contribution-welcome) to match these rules?

Right now, I made the guideline to follow current docstrings in Optuna as much as possible.
But, we still have some differences between the guideline and code.

Main differences may be:

1. Some functions or classes do not have docstrings.
2. Some docstrings do not include Example.
3. Some Examples do not follow the guideline.

If we need to fix them, we can make issues.

Does Note: do the same thing as .. note:: ?

yes, they are same.
Note: is google style, and .. note:: is sphinx style. I locally tested both Note: and .. note::, and they work as same.

[hvy]

If we need to fix them, we can make issues.

1 and 3 should be fixed but I don't think 2 should be considered an issue, i.e. examples are optional. Making issue(s) SGTM.

yes, they are same.

Thanks for the verification. Aligning to either seem fine.

[toshihikoyanase]

@keisuke-umezawa

__init__ method must be documented in __init__ method itself, not the class level docstring.

Could you tell me why you choose the method level docstring?
Currently, Optuna employs the class level docstring, and the Example Google Style Python Docstrings accepts it.

__init__ method can be documented in __init__ method itself, or the class level docstring.

In addition, could you show me an example repositories which employ the method level docstring?
I'm not familiar with it and some popular repositories such as TensorFlow and PyTorch use the method level docstring.

[keisuke-umezawa]

Could you tell me why you choose the method level docstring?
Currently, Optuna employs the class level docstring, and the Example Google Style Python Docstrings accepts it.

In addition, could you show me an example repositories which employ the method level docstring?
I'm not familiar with it and some popular repositories such as TensorFlow and PyTorch use the method level docstring.

Sorry, it seems that it is just my preference. I also checked other libraries s.t. sklearn, and they use class level docstring.

I intended to delete the arbitrarily choice to write args in either class level or __init__. In the google docstring guidelines, it says that we should choose one convention.

        The __init__ method may be documented in either the class level
        docstring, or as a docstring on the __init__ method itself.

        Either form is acceptable, but the two should not be mixed. Choose one
        convention to document the __init__ method and be consistent with it.

So, I choosed __init__ because it seemed natural for me at that moment. But I have realized it is not the standard in other libraries. So, we should document the __init__ method in the class level docstring. What do you think of it?

[hvy]

Sound perfectly reasonable to me!

[toshihikoyanase]

@keisuke-umezawa
I agree with you. Thank you for your detailed explanation!

[hvy]

@keisuke-umezawa, would you mind updating the coding style guide accordingly? Let me know if you're busy and maybe I could do it.

[keisuke-umezawa]

@hvy I changed as follows:
https://github.com/optuna/optuna/wiki/Coding-Style-Conventions/_compare/86d5c5cfa13bb0b88e8e5fef5a8d5402d6efe9cb...e9b2ca54e588d16eacc80e1cbd3ca4856b0c0c5b

[hvy]

Thanks a lot, the changes LGTM.

[keisuke-umezawa]

@hvy If you satisfied with the current guideline, we can close this issue. How do you feel about it?

[hvy]

Thanks, I'm personally okay with the new style. However, it'll require several changes (e.g. all the directives such as ..seealso: -> See also: etc.) so I'm wondering whether others don't have any comments.

[hvy]

Sorry for pinging but @crcrpar do you have any inputs on these changes?

[crcrpar]

Actually I don't have any take on how docstring should look like and I'm happy if there is one clear guideline.

[keisuke-umezawa]

Thanks, I'm personally okay with the new style. However, it'll require several changes (e.g. all the directives such as ..seealso: -> See also: etc.) so I'm wondering whether others don't have any comments.

I will add description that both style is acceptable for ..seealso and See also:.

[keisuke-umezawa]

I updated the guideline.

https://github.com/optuna/optuna/wiki/Coding-Style-Conventions/_compare/e9b2ca54e588d16eacc80e1cbd3ca4856b0c0c5b...97975720f757055c0897054e1538b7839e762b0b

[keisuke-umezawa]

I close this issue, because we achieved the goal I want to do when I opened it.
If you find any other issues about the guideline of docstring, please open another issue.