Refactor docstring params

[echedey-ls]

• Closes "default None" in docstring parameter descriptions considered harmful #1574 among other updates. See bottom of PR body.
• I am familiar with the contributing guidelines
• [ ] Tests added
• [ ] Updates entries in docs/sphinx/source/reference for API changes.
• [ ] Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
• Current code includes numpydoc compliant docstrings, examples, and comments everywhere.
• Pull request is nearly complete and ready for detailed review.
• Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

I just want the docstrings to be homogeneus and compliant with numpydoc.

Changes in this PR

1. Adds a space before colon in parameters and returns
   solar_azimuth: numeric -> solar_azimuth : numeric
2. Changes excessive use of parentheses in defaults
   return_components : bool (optional, default=False) -> return_components : bool, default False
3. Removes colon when default is to be specified
   logical_records : list or tuple, default: ('0100',) -> logical_records : list or tuple, default ('0100',)
4. default None -> optional ("default None" in docstring parameter descriptions considered harmful #1574)
5. See also -> See Also

[kandersolar]

Thanks for submitting this PR @echedey-ls!

I think the main priority for docstring formatting is that the sphinx HTML output is correctly rendered. Since spaces before colons and See Also versus See also don't make any difference in the HTML, I don't think those are really needed. The cost of changing them is that it clutters the git blame, which is only a minor problem, but still enough to want to avoid making these kinds of sweeping changes without a good reason for it IMHO.

For changing default None to optional, I think that is good. But probably some of the parameter descriptions should be updated as well. For example in pvlib.clearsky.detect_clearsky it now says:

    times : DatetimeIndex, optional
        Times of measured and clearsky values. If None the index of measured
        will be used.

Since None is no longer mentioned as the default value, I think the description needs to change to If not specified, ... instead. There are other cases like this too (e.g. surface_type in pvlib.irradiance.get_ground_diffuse and many parameters in pvlib.modelchain.basic_chain).

[echedey-ls]

Good points. I forgot to add an explanation to what I really want to address since I've been travelling a bit and didn't bother to check what I had written.

I intend to open a PR to hint contributors of docstring errors. I've come up to two options that may be useful. Both may be used at the same time:

1. suggest running the command python -m numpydoc --validate pvlib.module.func in the PR template and/or the contributing guidelines, so it can be tested quickly. From numpydoc validation, this section.
2. in the sphinx-build job (well, in the conf.py file). See this section of numpydoc.

Also, they recently added a pre-commit hook, although I feel that is a bit overwhelming to add to pvlib since it is currently not being used, I believe.

Anyway, for choice 1, it doesn't mind having these "issues" in the docstrings, since contributor would only apply the checker to their function/class/method... The problem with that is that I haven't found a way to include/exclude rules (it checks all of them) and that's definitely not desired. Some rules just don't apply to the pvlib style, like mandatory examples section, some punctuation or capitalisation, and users may not know of that. Adding a grep pipe to the output might be the hardcoded way to do so but it may be OS dependent.
Some work on this may be found at numpy/numpydoc#459

On the other hand, choice 2 allows to check for a custom subset of rules, so there is a cleaner output. However, it checks all sphinx docstrings. This PR would allow using the most of them that seem useful. For example, the "See Also" change allows using the rule that checks that all sections exists (it raises a warning for all of them). Adding a space before a colon and removing parentheses makes the default validator parse the argument name and the type correctly. To sum it up, most of the changes in this PR remove false positives and typos that mess with the validator, as a previous step. Personally, I like this option the most.

And these superfluous changes, well, since they almost don't have an impact on the docs, I thought of doing them via an independent PR. So checking another PR with the tool is not a pain (I'm aware this one is).

I will check the default None and clean those docstrings in the following days.

[kandersolar]

Thanks for this in-depth research @echedey-ls :)

One thing relevant especially for option 2 is that we don't actually use numpydoc, we use the napoleon sphinx extension (#691). I'm guessing that enabling both of them in the build would cause problems, but I don't know that for sure.

[echedey-ls]

I'm guessing that enabling both of them in the build would cause problems, but I don't know that for sure.

Good point, it does.

I will pursue choice 1, but I'd like to wait for the numpydoc issue to get resolved then.

Thanks for the review. Which changes should I keep? Changes 2 to 4 in the "changes" section from PR body?

[echedey-ls]

Continued work over #1828