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

Docstring default values for mutable arguments. #577

Open
vinRosso opened this issue Jun 14, 2023 Discussed in #527 · 3 comments
Open

Docstring default values for mutable arguments. #577

vinRosso opened this issue Jun 14, 2023 Discussed in #527 · 3 comments
Labels
extractor: griffe Related to griffe feature New feature or request

Comments

@vinRosso
Copy link

Discussed in #527

Originally posted by ZhiyuanChen January 29, 2023
Set default in arguments to mutable objects could cause horrible errors.
A general solution is to set default to None, and then assign correct default value in the function body.
Although the correct default value should be properly addressed in docs to avoid ambiguation.

However, it seems mkdocstring is not recognising the default values specified, like this:
image

Possible solution

Adding a Defaults section to the docstring could be the best solution so far.

def google_docstring(a, b=False, c=None):
    """ Description.
    
    Args:
        a (int): The first parameter.
        b (bool): The second parameter.
        c (list | None): The third parameter.

    Defaults:
        c: [1, 2, 3, 4]
    """

def numpy_docstring(a, b=False, c=None):
    """ Description.
    
    Parameters
    ----------
    a : int
        The first parameter.
    b : bool
        The second parameter.
    c : list | None
        The third parameter.

    Defaults
    --------
    c : [1, 2, 3, 4]
    """

def sphinx_docstring(a, b=False, c=None):
    """ Description.
    
    :param a: The first parameter.
    :type a: int

    :param b: The second parameter.
    :type b: bool

    :param c: The second parameter.
    :type c: list | None
    :default c: [1, 2, 3, 4]
    """
@pawamoy pawamoy added feature New feature or request extractor: griffe Related to griffe labels Jun 14, 2023
@pawamoy
Copy link
Member

pawamoy commented Jun 27, 2023

I've thought about it a bit more, and in fact I don't like having a new "Defaults" section. It would act as some kind of metadata section, that we would have to merge into other ones (Parameters, Other Parameters) and not render. So it would behave differently than every other section.

The only way I can see to parse a default value correctly within the parameter line is:

c (list | None, default=[1, 2, 3, 4]): The third parameter.

...and to compile that as Python code, to an AST, to fetch the default value of the default parameter, which is our default value for c. That means: no textual description for default values, only valid Python syntax.

We would not be able to do the same thing for Numpydoc as they allow arbitrary text for types. But it already has support for , default=, so that's OK (it's just that parsing that will not be correct in 100% of cases).

@vinRosso
Copy link
Author

I can see that it would be odd, since Griffe is already branching off from official docstrings documentations... But eventually "Defaults" would be an extra metadata section, such as the "See Also" and "Methods" sections that you mentioned in your numpydoc issue, right?.

Right now I'm also using arbitrary text to document some types in Google, as it's easier to read. So my worry is that compiling the whole thing as Python code would loose this advantage. I'm not a professional programmer, so there may be some better way to define a list of strings for example, but I hope that you get my point.

#e.g.
c (str[]): description`
c (list[str]): description

So I still think that adding the "Defaults" custom section would be the best way, as it's docstring type agnostic and allows for more freedom... But in the end, for me at least, it doesn't really matter what solution you're going to take, since anyone of them would be a nice extra feature to have, and I can adapt to whatever you're going to choose.

@pawamoy
Copy link
Member

pawamoy commented Jun 28, 2023

such as the "See Also" and "Methods" sections that you mentioned in your numpy/numpydoc#463 (comment), right?.

No, See Also and Methods sections can be rendered 🙂 They don't act like metadata.

Right now I'm also using arbitrary text to document some types in Google

Oh OK, it's true that it's not explicitly forbidden. Thanks for letting me know, I'll take that into account when working on the solution.

c (list[str]): description

Definitely the right way to annotate as a list of string 😄

But in the end, for me at least, it doesn't really matter what solution you're going to take, since anyone of them would be a nice extra feature to have, and I can adapt to whatever you're going to choose.

Noted, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extractor: griffe Related to griffe feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants