TST: Fail on warning
State: closed

Sets our pytest config to fail on unhandled warnings.

Fixes a bunch of tests to not fail.

This still has a few TODOs.

Closes https://github.com/pandas-dev/pandas/issues/16481
Closes https://github.com/pandas-dev/pandas/issues/19677
Closes https://github.com/pandas-dev/pandas/issues/13962
Closes https://github.com/pandas-dev/pandas/issues/22020

Labels:
name: Testing
description: pandas testing functions or related to the test suite

Comments:
Fixing all the warnings is definitely great! 

About harcoding `-Werror` in `setup.cfg` that can lead to various issues (cf   https://github.com/scikit-learn/scikit-learn/issues/12043 for more details). Adding those flags in CI setup might be more robust. Among other things hardcoding such a flag in `setup.cfg` may lead to the test suite failing (without good reason) with older or not yet released versions of dependencies,  when using PyPy or some other less common environments.
If it turns *all* warnings into errors, it might close #13962 as well. 
@rth thanks for the context, that's helpful. I think that elevating warnings to errors only on worker with the latest dependencies makes the most sense. I'll probably keep at this until things are passing, and then limit the elevation once things are passing.

> If it turns all warnings into errors, it might close #13962 as well.

Thanks for the link. It should close that.
There's a new warning coming through from numpy dev.

```
In [9]: pd.DataFrame(x).median(1)
/Users/taugspurger/miniconda3/envs/travis-37-numpydev/lib/python3.7/site-packages/numpy/lib/function_base.py:3267: RuntimeWarning: All-NaN slice encountered
  r = func(a, **kwargs)
Out[9]:
0    -0.601975
1    -0.572101
2     0.405565
3     0.848596
4     0.856747
5          NaN
6          NaN
7          NaN
8          NaN
9          NaN
10    0.333711
11    0.115346
12    0.193118
...
````

I assume the user should not see that warning?
All green, aside from the doc build timeout.

Are people generally OK with turning unhandled warnings into errors on the dev build? This will be warnings from

1. Python 3.7 (shouldn't be any new ones)
2. NumPy & python-dateutil dev
3. (most importantly) us

Does anyone expect to have a chance to review this today? The fact that I haven't seen a ResourceWarning turning into an exception suggests that we *may* have fixed that random failure on this branch. It's a lot of small changes, but hopefully harmless.
> Are people generally OK with turning unhandled warnings into errors on the dev build? This will be warnings from

So the errors will only happen at the numpy dev build (which is in allowed failures) ?

Is there some way to let the warnings from pandas itself to error always? (I think those would be fine to error on all builds, not only numpy-dev)
> So the errors will only happen at the numpy dev build (which is in allowed failures) ?

Yes, but I thought our policy was "allowed failures aren't allowed", but I've never understood why it's in the allowed failures section.

> Is there some way to let the warnings from pandas itself to error always?

Not that I know of.

edit: well, maybe. https://docs.python.org/3.6/library/warnings.html#warnings.filterwarnings takes a `module` argument. Does anyone know if that's the module where the Warning is defined, or the module where it's emitted from. Perhaps a 

```
filtewarnings =
    error:::pandas[.*]
```

works.
> but do we want to make a with tm.ignore_warnings(DeprecationWarning): ... utility context manager to replace the two-line with catch_warnings: simplefiltter("ignore", DeprecationWarning) ?

I hope that we wont really need `catch_warnings` going forward. Ideally tests are small / focused enough that a `pytest.mark.filterwarnings` is appropriate.
I wasn't able to get the suggestion of 

```filterwarnings =
error:::pandas[.*]
```

to work reliably. In my small test of

```
def test_warns():
    pd.Panel()
    pd.DataFrame({"A": [1, 2]}).ix[0]
```

the panel warning was not elevated to an error, but the `.ix` was. I didn't investigate too deeply.
Those interested in the ResourceWarning failures may want to take a look at https://github.com/pandas-dev/pandas/pull/22699/commits/cfb5ae2d4ac8b24b871da69f24e09960d8868cba. I tried to squash all the places we may have had unclosed files sitting around. If I had to guess, either https://github.com/pandas-dev/pandas/pull/22699/commits/cfb5ae2d4ac8b24b871da69f24e09960d8868cba#diff-0d7b5a2c72b4dfc11d80afe159d45ff8R389 or https://github.com/pandas-dev/pandas/pull/22699/commits/cfb5ae2d4ac8b24b871da69f24e09960d8868cba#diff-9d7323cc6b0cfa3f4482bc8d4268966dR213 is going to be the one that fixes it, but who knows.
I think I've addressed all the inline comments

Sorry to push on this relatively quickly, but I want to get our CI in a healthy state again. If possible, I'd like to merge this today.

I currently have two followup issues for warnings that need some care

1. https://github.com/pandas-dev/pandas/issues/22698 (Series ctor)
2. https://github.com/pandas-dev/pandas/issues/22712 (median with all-NA slice)

@jreback can you clarify our current policy on "allowed failures"? I don't recall why they're in the allowed-failures section.
> @jreback can you clarify our current policy on "allowed failures"? I don't recall why they're in the allowed-failures section.

- so numpy-dev is allowed to fail because there may be a change which breaks us but is not pushed to public (ideally would fail, except for when numpy / dateutil breaks things *everything* breaks and all builds would then fail)
- slow tests are there just to make the build green faster
- clipboard test uses to fail pretty unexpectedly
- doc build takes a long time.

so they are really there to get a probable green faster. I don't really have a problem moving them to required builds actually. 
Thanks. So I think it's best to treat numpy / dateutil dev as "allowed failures" but as soon as we see a failure we should open an issue that would block the next release (and maybe report upstream if it looks like an unintentional break).

We might consider moving the doc build to azure. Apparently the workers more powerful than Travis.
> Thanks. So I think it's best to treat numpy / dateutil dev as "allowed failures" but as soon as we see a failure we should open an issue that would block the next release (and maybe report upstream if it looks like an unintentional break).

yep, that's the idea

> We might consider moving the doc build to azure. Apparently the workers more powerful than Travis.

certainly
@TomAugspurger merge when you are ready
👍 pushed some doc fixes. Will merge on green.
Travis all passed. Merging.