DOC/CI: Fixes to make validate_docstrings.py to not generate warnings or unwanted output
State: closed

Follow up of #23514. Making `validate_docstrings.py` not generate warnings or other unwanted output, mainly when called with `--format=json`. This includes preventing matplotlib of opening windows with the plots, and also canceling output from flake8.

Also, fixed some bugs, and corrected some tests that weren't being run. 

The script should now be ready to be added to the CI (and to generate the json file with the docstrings state of the art).

- [X] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

CC @TomAugspurger 

Labels:
name: Docs
name: CI
description: Continuous Integration

Comments:
Hello @datapythonista! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`pandas/core/generic.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/pandas/core/generic.py) !

 - There are no PEP8 issues in the file [`pandas/core/indexes/base.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/pandas/core/indexes/base.py) !

 - There are no PEP8 issues in the file [`pandas/core/panel.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/pandas/core/panel.py) !

 - There are no PEP8 issues in the file [`pandas/core/strings.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/pandas/core/strings.py) !

 - There are no PEP8 issues in the file [`pandas/errors/__init__.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/pandas/errors/__init__.py) !

 - There are no PEP8 issues in the file [`pandas/plotting/_misc.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/pandas/plotting/_misc.py) !

 - There are no PEP8 issues in the file [`scripts/tests/test_validate_docstrings.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/scripts/tests/test_validate_docstrings.py) !

 - There are no PEP8 issues in the file [`scripts/validate_docstrings.py`](https://github.com/datapythonista/pandas/blob/a0547690c174d2d374609bede882c6c991327f22/scripts/validate_docstrings.py) !


Changes look good to me.

> The script should now be ready to be added to the CI (and to generate the json file with the docstrings state of the art).

Can you remind me what the end-goal of this is?
The json file is for me (or anyone else interested), to know what needs to be fixed in the docstrings.

The end goal is to have `./scripts/validate_docstrings.py` in the CI, which will fail if there is anything wrong in the docstrings (wrong documented parameters, formats, pep8 in examples...).

My next PR will be something like adding `./scripts/validate_docstrings.py --prefix=Series --errors=EX03`, and the fixes of pep8 issues in all the examples in Series docstrings, so the CI pass. And from here, keep adding until the 900 current errors are fixed, and everything is validated in the CI for new changes.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=h1) Report
> Merging [#23552](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/28a42da41ca8e13efaa2ceb3939e576d08c232c8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/23552/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #23552   +/-   ##
=======================================
  Coverage   92.24%   92.24%           
=======================================
  Files         161      161           
  Lines       51224    51224           
=======================================
  Hits        47254    47254           
  Misses       3970     3970
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.63% <ø> (ø)` | :arrow_up: |
| #single | `42.28% <ø> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.81% <ø> (ø)` | :arrow_up: |
| [pandas/core/panel.py](https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcGFuZWwucHk=) | `97.91% <ø> (ø)` | :arrow_up: |
| [pandas/plotting/\_misc.py](https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19taXNjLnB5) | `38.98% <ø> (ø)` | :arrow_up: |
| [pandas/errors/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2Vycm9ycy9fX2luaXRfXy5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [pandas/core/strings.py](https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3RyaW5ncy5weQ==) | `98.58% <ø> (ø)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.45% <ø> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=footer). Last update [28a42da...a054769](https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Great, that's what I thought.

On Wed, Nov 7, 2018 at 4:56 PM codecov[bot] <notifications@github.com>
wrote:

> Codecov <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=h1>
> Report
>
> Merging #23552
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=desc> into
> master
> <https://codecov.io/gh/pandas-dev/pandas/commit/28a42da41ca8e13efaa2ceb3939e576d08c232c8?src=pr&el=desc>
> will *not change* coverage.
> The diff coverage is n/a.
>
> [image: Impacted file tree graph]
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=tree>
>
> @@           Coverage Diff           @@##           master   #23552   +/-   ##
> =======================================
>   Coverage   92.24%   92.24%
> =======================================
>   Files         161      161
>   Lines       51224    51224
> =======================================
>   Hits        47254    47254
>   Misses       3970     3970
>
> Flag Coverage Δ
> #multiple 90.63% <ø> (ø) ⬆️
> #single 42.28% <ø> (ø) ⬆️
> Impacted Files
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=tree> Coverage
> Δ
> pandas/core/generic.py
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==> 96.81%
> <ø> (ø) ⬆️
> pandas/core/panel.py
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcGFuZWwucHk=> 97.91%
> <ø> (ø) ⬆️
> pandas/plotting/_misc.py
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19taXNjLnB5> 38.98%
> <ø> (ø) ⬆️
> pandas/errors/__init__.py
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2Vycm9ycy9fX2luaXRfXy5weQ==> 100%
> <ø> (ø) ⬆️
> pandas/core/strings.py
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3RyaW5ncy5weQ==> 98.58%
> <ø> (ø) ⬆️
> pandas/core/indexes/base.py
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5> 96.45%
> <ø> (ø) ⬆️
> ------------------------------
>
> Continue to review full report at Codecov
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=continue>.
>
> *Legend* - Click here to learn more
> <https://docs.codecov.io/docs/codecov-delta>
> Δ = absolute <relative> (impact), ø = not affected, ? = missing data
> Powered by Codecov
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=footer>.
> Last update 28a42da...a054769
> <https://codecov.io/gh/pandas-dev/pandas/pull/23552?src=pr&el=lastupdated>.
> Read the comment docs <https://docs.codecov.io/docs/pull-request-comments>
> .
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/23552#issuecomment-436808504>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIk7IGw6n7CT4Qfar2EanieNTVRloks5us2U5gaJpZM4YThEc>
> .
>

I agree that skipping is not ideal, but I think in this case is better than having the warnings, and I don't see other options.

But if you prefer, I can remove the `+SKIP`, not rendering with matplotlib, removing the output of flake8  and the minor changes with stdout/stderr and exit status are the important parts of this PR, getting those merge we can move forward with the automatic validation of docstrings.
It's fine for this one. I just think we might need to think of a better general way to deal with this. As there can also be genuine cases where you *want* a warning (showing a use case that gives a certain warning). It would be annoying to always have to skip those.
Totally agree. I think all the skips except the plot one are for deprecations. And personally I think it's reasonable to skip them since the deprecation until the removal.

The plot one would be probably worth investigating. But with all the work pending in the docstrings, I prefer to skip that test, and move forward with all the fixes.

Let me know if this can be merged, or if there is anything that needs to be changed.