NREL / flasc

A rich floris-driven suite for SCADA analysis

https://nrel.github.io/flasc/

BSD 3-Clause "New" or "Revised" License

32 stars 18 forks source link

Feature/add docstrings #197

Closed paulf81 closed 3 months ago

paulf81 commented 4 months ago

Add docstrings, and docstring checks to FLASC

FLASC has functions currently missing docstrings, this is a problem given automatic documentation with sphinx relies on those strings.

Following FLORIS, we use the google style guide (https://google.github.io/styleguide/pyguide.html) but this is not uniformly followed so this PR tries to get higher uniformity

This pull request:

Updates RUFF to check for google-style docstrings
Makes running RUFF part of CI
Adds missing docstrings to functions
Makes all docstrings google-style compliant
- Adds summary line
- Focus on Args/Returns style
- Only list type in return
- Make sure all arguments appear in docstring
Add module level docstrings
Standardize on class / init docstrings a la: https://google.github.io/styleguide/pyguide.html
Fix spelling erros in docstrings where they appear

paulf81 commented 4 months ago

@misi9170 , this all grew a a bit as I got going and what small cheat I did was to add a few files that I wasn't sure we are keeping long term to the excluded list of docstring checks in pyproject.toml, but I think it's basically ready for review

paulf81 commented 4 months ago

Ok @misi9170 I made some final corrections think this now ready for review

paulf81 commented 4 months ago

Thank you for these comments @misi9170 , I've tried to make all revisions but seems likely things could still be a little messy in places. I did find that the the saying a function returns a Tuple is the preferred google style so tried to update that everywhere.

misi9170 commented 4 months ago

@paulf81 Thanks. My feeling here is that we shouldn't necessarily aim for perfection with this PR, so I'm going to approve it.

Thanks for looking into the recommended style for the list of Returns. To be honest, I don't really agree with the recommendation of listing each multiargument return as a tuple---I think it's clearer to just list our each of the returns, as done with the Args, since as far as I'm aware, a tuple is always returned from a python function if there are multiple outputs.

As it stands, we have a mix of both of these styles for the Returns: statements. I'm OK to just leave it as is for now if you'd like, so that we can move forward with the PR.