NumPy docstyle should denote "optional" arguments

[bdice]

Description

@cbkerr noted in a PR review that I didn't use the optional flag for optional arguments. I think this problem exists all throughout the new NumPy-style docs and should be dealt with before finishing #315. I'm creating a separate issue because I would perform this as a "last step" after all docstrings have been converted.

Proposed solution

It should be relatively easy to detect optional arguments because they should all have something like "(Default value: x)" in the docstring. Of course, it's possible some docstrings are missing that and should be updated. While this is happening, it would be good to standardize on a few formatting things. We have some mixture of:

1. Periods before and after the parentheses (I prefer after).
2. Code font for the default value (I prefer code font for anything except False, True, or None, including strings with single quotes like 'default_string').
3. Wording: Defaults to x. vs. (Default value = x)

I think my preferences match the majority of the code base already:

    parameter_name : bool, optional
        Description of parameter (Default value = True).
    another_parameter : str, optional
        Description of parameter (Default value = ``'hello world'``).

Also, I find this ambiguous and don't know what to do: we have many places in the code where we define a default value as None but then use a function or other non-mutable argument like [] or {} if the provided value is None. What is the right way to document this? Do we say the default is actually None or do we say the default is whatever replaces None? In my recollection, we're specifying the replacement value for functions and strings but not for empty collections (which may be accepted as an implementation detail).

    function_parameter : callable, optional
        Function used to copy data (Default value = :func:`shutil.copytree`).
    list_parameter: sequence of :class:`~signac.contrib.job.Job`, optional
        Sequence of jobs to use (Default value = ``[]``)

[vyasr]

In my opinion, the default should be the actual default (e.g. None), but the docstring should describe what happens in that case. For example: "If None, files are copied using :func:shutil.copytree"

[csadorf]

I agree with Vyas. The formalized default value should be the one that is actually matching the function signature, but the description should mention what the code logic will then default to. I think it's important to mention the "actual" default value, so that users have the option to explicitly provide it if they want to.

[kidrahahjo]

Can I work on this one after #354 is merged to numpy_docs?

[bdice]

I spoke with @Carreau recently and used a tool he's developing (https://github.com/Carreau/minirst/) which might help automate this process by detecting which arguments are optional and proposing that change. I opened an issue to request this feature: https://github.com/Carreau/minirst/issues/11

It may be less work to implement that feature than to check all the docstrings by hand.

[bdice]

Can I work on this one after #354 is merged to numpy_docs?

@kidrahahjo Yes, of course! Let's wait until #354 is merged to decide how to prioritize our next steps. Perhaps we can set up a call and discuss automated tooling with @Carreau (though we're not ready quite yet).

[kidrahahjo]

@bdice Since #354 is merged, how should we proceed to resolve this issue? (Just curious to know :sweat_smile: )

[Carreau]

I'm still working on som edge case with my autoreformatter. FOr example it suggests:

     Returns
     -------
-    NoneType or :class:`~signac.sync.FileTransferStats`
+    NoneType or : class:`~signac.sync.FileTransferStats`
         Returns stats if ``collect_stats`` is ``True``, else ``None``.

Which is definitely wrong, but also mean the numpydoc likely passes that incorrectly. Right now it needs an unreleased version of Numpydoc to function;

I'll send a pr to see what it finds.

[bdice]

@Carreau Please let us know if there are any particular edge cases where you'd like help! Thank you for testing your tool with signac, it is a great help. 👍

[vyasr]

@Carreau let me echo @bdice's thanks, any auto formatting is welcoming :) any updates on this? Totally fine if not, I'm trying to assess open targets for our next release and whether to plan to include this or whether we should move this to the next milestone.

@bdice @mikemhenry in the event that this PR will take a bit longer, I think we can safely punt #344 to our v2.0 release. While it would be ideal to have optional arguments documented consistently, if there's a likelihood of getting @Carreau's autoformatting to work we shouldn't waste developer time on fixing the docstrings manually, and signac provides him a nice test case for his tool. Conversely, I don't think the documentation issues are significant enough to merit holding off on a release (I'm sure there are other typos and minor errors introduced by our numpydoc rewrite that will be most easily caught if we just release and see what people find).

[Carreau]

Sorry, no I haven't had time to move forward on it, it will require to rewrite an RST parser with more flexibility than docutils. I'm slowly working on it (not at all in the last two weeks). But I'll need such a thing for another project for which I'm trying to get funding.

[vyasr]

No problem, I'll move this to a later milestone for now, then.

[mikemhenry]

I'm fine with punting this to v2

Also thanks @Carreau for the work on auto formatting!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[vyasr]

If we don't have a tool available we probably should tackle this manually before v2. This would be a good first issue for someone.

[vyasr]

This was resolved in #763 (on next, which is why this issue didn't get closed at the time).