Fill in default values automatically in documentation

[jgosmann]

Motivation and context:
This PR adds a Sphinx extension that automatically fills in the defaults in the function signatures for parameters. At this point I'm mostly looking for general feedback, there might be some details that would need some discussion and some things that should be done before this PR gets merged (if it gets merged).

The advantages of this approach:

• One has to write less boilerplate within the documentation.
• Inconsistencies between the actual parameter values and documentation are avoided.
• It would allow us to replace numpydoc with sphinx.ext.napoleon as mentioned here which would cut down on an external dependency. Also sphinx.ext.napoleon resolves a bug with numfig that I encountered in Nengo SPA.
• The default value gets documented in the function signature as it is usually done in Python docs.
• The default is not documented in the field that is supposed to be for the data type.

Potential disadvantages:

• The default value might be overwritten by a config. That is not evident from the function signature (but also not that evident from our current notation).
• The extension relies on the argument having the same name as the parameter. This allows for wrong values being documented as defaults if an argument and parameter names do not match up. Though that would likely be confusing code that should be avoided ...

Interactions with other PRs:

Currently based on #1349, but doesn't really depend on it and could probably easily be rebased onto master.

How has this been tested?
build the documentation and looked and the function signatures

How long should this take to review?

• Average (neither quick nor lengthy)

Types of changes:

• New feature (non-breaking change which adds functionality)

Checklist:

• I have read the CONTRIBUTING.rst document.
• I have updated the documentation accordingly.
• I have included a changelog entry.
• [n/a] I have added tests to cover my changes.
• I have run the test suite locally and all tests passed.

Still to do:

• The default of the function argument of nengo.Connection cannot be resolved at the moment.
• Go through the documentation and remove manually documented defaults.

[jgosmann]

This does not yet allow us to replace numpydoc because of sphinx-doc/sphinx#2549, but when that is resolved it should be possible.

[Seanny123]

Checks are failing, but only due to unrelated problems.

[Seanny123]

Do you want to clean up the commit history (and rebase?) before this is reviewed? Also, how should this be reviewed? I'm assuming one should try to build the documentation, as well as check the code in the individual commits?

[jgosmann]

I could do a rebase, I don't think there is much to cleanup in the history. But do you want to review #1349 first? This PR is based on the other one. I would recommend going by commit, looking at the built documentation also makes sense.

[jgosmann]

It would be nice to get this reviewed and merged, so I can use it in the Nengo SPA documentation. This implies that it would be nice to have #1349 reviewed and merged first.

[tbekolay]

@jgosmann could you rebase this to master now that #1349 is merged? Or I can do it if you'd like. From a quick look, it looks good, though I think I'd rather the sphinx extension stuff be put in docs/conf.py..

I'm also not super sure about removing the defaults from the docstrings. The docstrings might still be used in the context of, e.g., help(nengo.Ensemble) or nengo.Ensemble? in Jupyter, so having defaults listed there seems helpful. It might be possible to modify the docstrings through metaclass stuff, or by making __doc__ a property (which would allow you to inspect the current live default also), but that can be left to a future PR. For now I think it's best to just not remove the information that's already there.

[Seanny123]

FYI, Jan is AFK until Sunday.

[tbekolay]

Ah thanks for the heads up! I'll do the rebase for now and leave the rest for Jan when he's back.

[jgosmann]

For now I think it's best to just not remove the information that's already there.

That pretty much cancels all the advantages of this PR except for one (“The default value gets documented in the function signature as it is usually done in Python docs.”).

[tbekolay]

That pretty much cancels all the advantages of this PR except for one (“The default value gets documented in the function signature as it is usually done in Python docs.”).

I thought that was the point of the PR

[jgosmann]

It is one of the points, but I listed four other reasons at the top.

Some more thoughts: help(...) is for interactive usage, but it is easy to lookup a default in interactive mode (e.g. nengo.Ensemble.radius.default). Though maybe that method should be added somewhere to the doc strings. This still doesn't really help with other tools reading and displaying the doc string. For those we might need to modify the doc string which is likely to be annoying because the text has to be parsed etc. and Sphinx should still read the original doc string.

[tbekolay]

Most users will not know to do something like nengo.Ensemble.radius.default, though to be fair they probably also won't know to use help and will look in the docs, so if anyone else has an opinion as to whether the defaults should be left in or take out of the docstrings, feel free to chime in.

If removing the defaults from the docstrings is a must-have for this PR @jgosmann then I'll look into the docstring modification approach.

[jgosmann]

If removing the defaults from the docstrings is a must-have

Maybe not a must-have, but I'd find it really nice. If we were to keep the defaults in the docstrings, could we at least move them out of the type definition into the parameter's description text?

[tbekolay]

could we at least move them out of the type definition into the parameter's description text?

We could, why do you prefer it there out of curiosity?

To explain my reasoning for putting it there in the first place, I put it in the type description so that it's always obvious where it is; I find in the NumPy documentation and other places they will say "optional", but it's not always clear where to find the default. Usually I want to know the default when I see the word "optional" so I thought it made sense to be right beside it.

Other ways to be consistent didn't feel great to me. E.g., you could have it at the start, but that way overemphasizes the default when most people just want to know what the kwarg does:

    radius : int, optional
        Defaults to 1.0. The representational radius of the ensemble.

It just reads weird. "This defaults to x. By the way, this is ..."

Putting it at the end is okay:

    radius : int, optional
        The representational radius of the ensemble. Defaults to 1.0.

But visually it is often at an inconsistent place because the previous sentence could end anywhere on that line; i.e., it's hard to visually scan this for defaults:

    radius : int, optional
        The representational radius of the ensemble. Defaults to 1.0.
    normalize_encoders : bool, optional
        Indicates whether the encoders should be
        normalized. Defaults to True.

If you manage to not let any inconsistent examples slip through, you can enforce that all "Defaults to x" are on a new line, or a new paragraph if you want it to be scannable in the rendered docs:

    radius : int, optional
        The representational radius of the ensemble.
        Defaults to 1.0.
    normalize_encoders : bool, optional
        Indicates whether the encoders should be normalized.

        Defaults to True.

But the downside of this is that defaults now always take up at least an entire line (possibly two), where putting it in the type field often results in not using a new line for the default.

Another reason that I am not a fan of having it in the long-form description is that the description is meant to be read like a normal English sentence. Consistency isn't the norm for English, so you will often get people putting things like this:

    normalize_encoders : bool, optional
        If True, which is the default, encoders will be normalized.

And that's impossible to scan. I have to read the whole description every time to find the default.

I guess the main underlying question is whether defaults are something that are important enough to be easily scannable. I would argue yes, but if the consensus feels otherwise, then that's fine.

[jgosmann]

I agree with your points and there is probably not a perfect answer.

The reasons why I dislike the default in the type definition: The field is intended for the data type and only the data type (plus optional where it applies). Sphinx/Numpydoc/napoleon try to parse that field accordingly and other things in there can will also be interpreted as data types. I believe that this currently does not visibly show, but it might in the future. It also shows when switching from Numpydoc to Napoleon (which is shipped with Sphinx and is what is used for Nengo SPA as it solves another problem). See this comment for details.

Putting the default into the description is also in line with what the numpydoc docstring guide says.

[jgosmann]

Because no one has replied in two weeks: I stated my arguments and I am aware that either option (putting the defaults into the documentation or not) has advantages and disadvantages. I'm willing to go with whatever the majority thinks is best (or as no one else commented so far, what @tbekolay prefers considering my arguments). Maybe it also helps to get an estimate how much the Sphinx documentation is used vs. other forms to access the documentation. I certainly usually use the Sphinx documentation or look up the source code itself (where the default is stated as part of the parameter definition, so I do not require it to be repeated in the doc string). Sometimes I use the doc string in Jupyter Notebook or Vim, but mostly to remind myself of the function signature, not so much to look up default values. But then I am using Nengo quite a bit and know many of the defaults by heart which does not apply to novice users.

[tbekolay]

I think I've mellowed a bit on having the defaults in the docstring itself and so am OK with moving forward with this PR since the only strong opinions seem to be from me and Jan. Having the default automatically inserted and always in sync is likely worth the potential confusion for people using help or ? in Jupyter (which is likely rare anyhow). I'll look at this in more detail tomorrow.

[jgosmann]

Friendly reminder as this would be nice to have for Nengo SPA where some documentation related PRs are in queue anyways.

[drasmuss]

Moved the sphinx logic to nengo/nengo-sphinx-theme#33, and updated this PR to use that (pending a new nengo-sphinx-theme release). Also rebased and fixed up a few minor things that have changed since last we looked at this.

[hunse]

One question re using help: I typically have an IPython terminal running and call help routinely on functions from other packages to read documentation. In Numpy/Scipy, when you call help on a function, the function signature appears at the top with the defaults. Would this be the case for us?

Another minor potential downside: Some parameters do not have defaults that are obvious from the signature. For example, a parameter with the empty dict {} as a default will usually have None as the default in the signature, then fill the dict in later, so that we don't accidentally share one dict across multiple instances. This would make it less clear in these situations what the default actually is. That said, these situations tend to be few (I think our use of FrozenTypes has allowed us to put more classes in the signature itself).

At first I was hesitant about this, but I think I'm ok with it. It is how Numpy/Scipy does it (as far as I can tell), and I've typically been happy with their documentation, both when I access it online and through help.

[jgosmann]

One question re using help: I typically have an IPython terminal running and call help routinely on functions from other packages to read documentation. In Numpy/Scipy, when you call help on a function, the function signature appears at the top with the defaults. Would this be the case for us?

Without having tested it, I believe this would not be the case. I assume help shows the raw docstring, whereas this a Sphinx extension that is only run when building the docs with Sphinx.
However, when running Python interactively, you have more options to interact with the code and that should allow you to retrieve the default value (of course it is not as convenient if it is directly in the documentation).

Some parameters do not have defaults that are obvious from the signature.

I believe that his is pretty rare in the Nengo code base. I cannot think of any parameters that has an empty dictionary as default. Maybe there are some with an empty list ...

[drasmuss]

Updated for the new nengo-sphinx-theme release, this looks good to me.