DOC: Fixes to docstrings, mainly PR09 errors
State: closed

closes #28602

Labels:
name: Docs

Comments:
Hello @HughKelley! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



























There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-12-31 19:55:57 UTC
does this completely close #28602?
> does this completely close #28602?

yes, after merging, running `python ./scripts/validate_docstrings.py --errors=PR09` should not return any errors and we can add that error to the CI checks (Which I'd like to try to make a PR for if it's alright). 
hit the wrong button in the comment dialogue and closed by accident
@HughKelley in `ci/code_checks.sh` we validate that errors in docstrings don't have. Can you add `PR09` to the list of errors we check, if this is the last batch. This way future changes to docstrings won't add new PR09 errors (the CI will fail).
Ups, just seen your previous comment now. Please better add the check here.
That's a false positive, and I think it's fixed in the numpydoc version of our script. I expect to have the PR that replaces our script by the numpydoc version in few days, so don't worry about those errors, you can leave the docstrings correct.
I've fixed the whitespace errors I made in previous commits. Just waiting to see if I successfully merged recent commits to master and whether the CI checks will be green since there may be PR09 errors in the code again depending on the status of the ci/code_checks.sh scripts. 
@HughKelley seems something went wrong with your latest update with master (there are too many commits included here, giving a big diff). 

Normally something like

```
git checkout PR09_batch_5
git fetch upstream
git merge upstream/master
git push origin PR09_batch_5
```

should do the trick
>  something went wrong with your latest update with master 

definitely. I used rebase instead of merge. 

I think I'm going to close this PR since it's a mess, cherrypick what I need into a new branch and open a new PR once the CI checks are all passing.

Unless you'd rather I do something else.

Thanks for the help/patience as I figure this stuff out.
I think that the snippet I posted should still be able to clean it up (even after the rebase), but if that doesn't work out, no problem in creating a clean branch / PR! 
(BTW, you can actually create a new branch locally but still push to this remote branch name / update this PR, you only then need to force push)
The first section in this post describes what I'd do: https://datapythonista.github.io/blog/useful-git-commands.html
@datapythonista @jorisvandenbossche these checks are green. 

Joris, the indentation error you pointed out is fixed. 

Am I responsible for resolving the merge conflict? Should I update the PR with the most recent changes to master and let the checks run again?

Thanks!

Yes, you need to resolve the conflicts before we can merge. `git fetch upstream` and `git merge upstream/master` should get the conflicts locally, so you can fix them. Thanks! 
will do, thanks
@datapythonista thanks for reviewing. The checks do not pass as there are still PR09 errors in the code. 

Has the update to the numpydoc script occured yet? Is there an open issue or PR for that I can keep an eye on?

The script doesn't recognize the `.` at the end of the description of `method` in `pandas.DataFrame.corr`. 


I've been working in using the numpydoc script, but there is still work to do, and I'm travelling, so it'll take some rhinestone to be ready.

We can merge this in the meantime, but you should undo the changes to the comment in `code_checks. sh`.

Also, please have a look at what is broken in the CI, so we can merge. 

Thanks! 
@HughKelley do you have time to address the last comments, so we can get this merged? Thanks!
> @HughKelley do you have time to address the last comments, so we can get this merged? Thanks!

@datapythonista 

Thanks for following up. Sorry this wasn't more of a priority for me during the holidays. 

i'm actually cherry picking stuff into a new (cleaner) PR that I'll get merged in the new year. 

thanks very much for everyone's work on the package and helping me as i learn this stuff.