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

Migrate to established doc style #31044

Open
tobiasdiez opened this issue Dec 12, 2020 · 15 comments
Open

Migrate to established doc style #31044

tobiasdiez opened this issue Dec 12, 2020 · 15 comments

Comments

@tobiasdiez
Copy link
Contributor

tobiasdiez commented Dec 12, 2020

It's worth considering to migrate from Sage's custom documentation style to one of the more established/standarized ones such as:

def function(param1: int, param2: str) -> bool:
    """
    Example function doing nothing.


    Parameters
    ----------
    param1
        The first parameter.
    param2
        The second parameter.
        The description may span multiple lines. Following
        lines should keep the indention.

    Returns
    -------
    bool
        True if successful, False otherwise.

    Examples
    --------
    Examples should be written in doctest format, and should illustrate how
    to use the function.

    >>> print([i for i in example_generator(4)])
    [0, 1, 2, 3]

    """

The return type must be duplicated in the docstring to comply with the NumPy docstring style.

def function(param1: int, param2: str) -> bool:
    """
    Example function doing nothing.

    Args:
        param1: The first parameter.
        param2: The second parameter.
            The description may span multiple lines. Following
            lines should be indented.

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

    Examples:
        Examples should be written in doctest format, and should illustrate how
        to use the function.

        >>> print([i for i in example_generator(4)])
        [0, 1, 2, 3]

    """

More examples for numpy and google can be found at https://www.sphinx-doc.org/en/master/usage/extensions/napoleon.html

Doctrings in these styles can be validated by https://pypi.org/project/pydoctest/

CC: @mkoeppe @jhpalmieri

Component: documentation

Issue created by migration from https://trac.sagemath.org/ticket/31044

@jhpalmieri
Copy link
Member

comment:1

More examples here: https://www.sphinx-doc.org/en/master/examples.html

I like that the inputs and outputs are highlighted in our style, whereas some others are more freeform. The numpy style appeals to me for this reason. The standard Python documentation is another model we could use, and its familiarity might be an advantage.

We are also not using the default Sphinx setup except in a few spots (like simplicial complexes: https://doc.sagemath.org/html/en/reference/topology/sage/topology/simplicial_complex.html#sage.topology.simplicial_complex.Simplex). We could head in this direction, too.

@tobiasdiez
Copy link
Contributor Author

comment:2

I agree, numpy is quite readable. It's a bit sad though that you have to specify the return type in the documentation string, so it's not enough to provide it as a typing annotation.
Google doesn't have this disadvantage and also requires the grouping of paramaters and return value etc. NumPy style tends to require more vertical space, whereas Google style tends to use more horizontal space. I guess with modern widescreen monitors that might be an advantage of the google style.

I've updated the ticket description with examples (essentially taken from the napoleon docs).

What would be the right way to move forward with this? I guess at some point there should be a vote on the developer mailing list.

@tobiasdiez

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

I don't understand the quote "Google style tends to be easier to read for short and simple docstrings, whereas NumPy style tends be easier to read for long and in-depth docstrings" from the napoleon web page. I don't care that much about which is easier to read in raw form — I care more about how they are rendered by Sphinx, and we should have a lot of control over that. Or am I missing something? If the Sphinx rendering varies for the various styles, then it would be great to have examples of the rendered documentation, not just the raw code.

@tobiasdiez
Copy link
Contributor Author

comment:4

Both are rendered exactly the same. Napoleon converts the google or numpy format into restructured text with the correct annotations (like :param:) and then you can feed it into autodoc, where it is processed by spinx and displayed correctly. So the decision google vs numpy vs annotated-restructured is only about which form we as the developer want to write.

The display/rendering format can indeed be customized very easily (in a second step, after it's properly parsed by autodoc).

@mwageringel
Copy link

comment:5

Replying to @tobiasdiez:

Both are rendered exactly the same. Napoleon converts the google or numpy format into restructured text with the correct annotations (like :param:) and then you can feed it into autodoc, where it is processed by spinx and displayed correctly.

Would it be an option to implement this for the style Sage currently uses? I would expect this to be more manageable than changing every docstring to a different style.

@tobiasdiez
Copy link
Contributor Author

comment:6

Replying to @mwageringel:

Replying to @tobiasdiez:

Both are rendered exactly the same. Napoleon converts the google or numpy format into restructured text with the correct annotations (like :param:) and then you can feed it into autodoc, where it is processed by spinx and displayed correctly.

Would it be an option to implement this for the style Sage currently uses? I would expect this to be more manageable than changing every docstring to a different style.

It's certainly possible but napoleon's parsing is already quite involved and we would need to duplicate that.

By using a standard convention for docstrings we would also get all the other advantages that come with following standards like:

The migration can also be very fluent; for example, if one introduces a new method or change an existing one then one uses a standard docstring format. Thus, over time the codebase gets slowly migrated.

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:8

Related: #30884, #30893

@jhpalmieri
Copy link
Member

comment:9

Currently our "TESTS" section does not appear in the documentation unless --include-tests-blocks is passed to the docbuilder. How can we reproduce that with a different format? Maybe use napoleon_custom_sections to create a "TESTS" section?

@tobiasdiez
Copy link
Contributor Author

comment:10

Replying to @jhpalmieri:

Currently our "TESTS" section does not appear in the documentation unless --include-tests-blocks is passed to the docbuilder. How can we reproduce that with a different format? Maybe use napoleon_custom_sections to create a "TESTS" section?

Yes, that should work.

@jhpalmieri
Copy link
Member

comment:11

I think that a big obstacle to this is #30893. I tried adding 'sphinx.ext.napoleon' to our Sphinx extensions and ran into various error messages.

@tobiasdiez
Copy link
Contributor Author

comment:12

Yes, this will probably require a few changes to our doc builder. I would be willing to work on this, but before starting this project we should decide if we indeed want to migrate to a different docstyle. Should I start a discussion on the dev list, or what would be a good process?

@jhpalmieri
Copy link
Member

comment:13

Ideally, we could start to use a new docstyle while the old one still functions, and then people could see samples within the Sage library of both. Regardless, the Sage community needs to make two choices: do we want to switch to a different default format, and if so, which one?

So maybe the right thing to do is to start a discussion on sage-devel about it. If there is general support, then the next steps would be to work on #30893, enable napoleon, and convert a few files to the new format. Maybe try to write a script to convert the current format to another one and apply it to parts of the Sage library, say one directory at a time.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 7, 2023

https://docformatter.readthedocs.io/en/latest/index.html can be used for reformatting docstrings to style guide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants