Add initial property-based tests using Hypothesis
State: closed

This pull request has no features or bugfixes for Pandas itself (though it does have a whatsnew entry for contributors), but adds support for and actually adds a few tests using Hypothesis.  The new section of the contributing guide below should explain what this means and why it's desirable.  There are basically three contributions here:

1. Contributor documentation and all the dependency setup to support Hypothesis tests in CI.
2. A pleasantly concise pair of passing tests in `test_ticks.py`.
3. Three *failing* tests in `test_offsets_properties.py` based on #18761 by @jbrockmendel.  I've made these idiomatic from the Hypothesis side, but while it seems likely that at least `test_on_offset_implementations` might be finding real bugs fixing them is beyond me at the moment.  I thought it worth including them anyway, with an `xfail` decorator.

Future work on Hypothesis tests could focus on such fruitful areas as round-tripping data via any of the serialisation formats Pandas offers (I tried to write a demo of this and couldn't get it to pass), or the melt and merge tests that @TomAugspurger mentioned.

Closes #17978, after which more specific issues could be opened with suggested tests.

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

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22280?src=pr&el=h1) Report
> Merging [#22280](https://codecov.io/gh/pandas-dev/pandas/pull/22280?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/55d176d1cd624f8921379c0ceed06fb8e00625b9?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `0%`.

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

```diff
@@            Coverage Diff             @@
##           master   #22280      +/-   ##
==========================================
- Coverage   92.04%   92.03%   -0.01%     
==========================================
  Files         169      169              
  Lines       50776    50780       +4     
==========================================
  Hits        46737    46737              
- Misses       4039     4043       +4
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.44% <0%> (-0.01%)` | :arrow_down: |
| #single | `42.22% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22280?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/\_tester.py](https://codecov.io/gh/pandas-dev/pandas/pull/22280/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX3Rlc3Rlci5weQ==) | `23.8% <0%> (-5.61%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22280?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/22280?src=pr&el=footer). Last update [55d176d...779b49a](https://codecov.io/gh/pandas-dev/pandas/pull/22280?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

This looks really nice, definitely prettier than #18761.

Do the xfailed tests actually fail or is that just being defensive?  Ideally we'd like to have `strict=True` in xfails.
Thanks!  I have an an unfair advantage in Hypothesis idioms, but I couldn't have written anything like this without your example to follow on the Pandas side :smile: 

The tests really do fail - I've added `strict=True` to the xfails and a note to one which is inconsistent.  Hopefully someone who knows more about Pandas internals than I do can either improve the tests or actually fix the bugs!
Agreed this looks quite nice, thanks @Zac-HD .

For better or worse, our test dependencies are all optional other than pytest. I think hypothesis should follow that pattern. If others agree with that policy, I suppose the easiest way to do that would be to keep tests using hypothesis in a separate file, and `pytest.importorskip('hypothesis')`, or add a helper in `pandas.util.testing`.

I'll look at those xfailing tests later.
In principle I don't have any particular objections to making Hypothesis optional and keeping it in particular files - that's how it works for Xarray, and creating a new sub-directory `tests/hypothesis/` isn't hard.

The only catch is that they should be executed as part of *every* test run in CI!  Otherwise you miss all the platform-specific issues that it could turn up, including the thing where `test_on_offset_implementations` has to be marked `strict=False` because it only fails on *some* operating systems.

If you feel that property-based tests are going to be valuable - and there are obvious applications to testing e.g. round-tripping serialization that I couldn't get to pass - personally I'd just make Hypothesis a lightweight required dependency for development.  Happy to let you decide either way though!
@TomAugspurger : I think `hypothesis` could be quite valuable for testing purposes.  However, I would play conservative first and make this optional as you suggested.  If it proves itself to be very important for testing purposes, then we could consider making it required later on.
OK, optional it is then! Do I just list the dependency in `ci/requirements-optional-conda.txt`?  Or is there something more involved?  

And just to make this *more* fun, it seems like the conda-forge package doesn't work for Python3 under OSX... cc @jochym for help please 😨 
@Zac-HD namespace collision I think :/ Seems to be `hypothesis-python`, at least on conda-forge.

Our CI setup is a bit messy, but I think

1. Include `hypothesis-python` in `ci/environment-dev.yaml`
2. Include a rename in `scripts/convert_deps.py` from `hypothesis-python` to `hypothesis`
3. Run `python scripts/convert_deps.py` and stage / commit the files
4.  Update some of the CI files to include `hypothesis-python`
  a. circle-27-compat.yaml
  b. circle-36-locale.yaml
  c. travis-27.yaml
  c. travis-35-osx.yaml
  d. travis-37.yaml
  e. appveyor-27.yaml
  f. appveyor-36.yaml

Hmm, I've just checked and I don't think the name is the issue after all (and I can't find a package called `hypothesis-python` on conda-forge at all).

And it's *currently* listed in `ci/environment-dev.yaml` - do you mean moving it to `ci/requirements-optional-conda.txt`?
> Hmm, I've just checked and I don't think the name is the issue after all

Ah, sorry I misread the travis output. Ignore that.

> And it's currently listed in ci/environment-dev.yaml

That's perfect. Could you also run `python scripts/convert_deps.py` and include the output (for our contributors using pip).
@Zac-HD Is there anything else from #18761 worth trying to salvage?
@jbrockmendel - it looks like you had some neat ideas for further tests of e.g. timedeltas, but I thought it would be better for me to stick to the groundwork and leave them for a follow-up PR that could do them justice.  

If you or anyone else would like to work on that, I'd be happy to advise on Hypothesis idioms (a standing offer for any open source project).
@jreback - I've implemented all the changes from your review.  Is there anything else I need to do before this can be merged?
Ping @TomAugspurger & @jreback for a final review.  It would be great to iterate on this at the PyCon AU sprints if it can be merged by next weekend, so if there's anything left to do I would really like to hear about it soon!
Merged master. The travis timeout should be fixed now.
let me have a look
@jreback my and Tom's view is that we're best off with Hypothesis as a hard dependency to run tests.  Can you live with this and give an approving review?  If not, what problems do you see?

(if possible I'd really like to merge this Friday or Saturday so I can talk about it at PyCon Australia and get some follow-up work done at the sprints)
it still can be a hard dependency for our testing

but as i said users code will immediately fail if they only have pytest

for user code that’s all that we can / should require 

I am not sure why hisbis a problem
you simply put a skip if in the conftest itself
if it’s not installed
What's the issue with having it as a required test dependency? Having to write tests around that constraint seems unnecessarily burdensome on contributors / maintainers.
I guess its ok to add as a dep for testing. can you update the install.rst as well.
@TomAugspurger - I'm quite certain these build issues are unrelated to the pull.  Any idea on how to make it green?
Hopefully fixed by https://github.com/pandas-dev/pandas/pull/22499

On Fri, Aug 24, 2018 at 8:09 AM Zac Hatfield-Dodds <notifications@github.com>
wrote:

> @TomAugspurger <https://github.com/TomAugspurger> - I'm quite certain
> these build issues are unrelated to the pull. Any idea on how to make it
> green?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22280#issuecomment-415753565>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIumRZOu3KXEcutZxC_uidGika65xks5uT_r0gaJpZM4V5KSi>
> .
>

:tada: - it's working!  @TomAugspurger, if you (or anyone else!) can hit 'merge' by, uh, 4am UTC that would be in time for my talk at PyCon Australia :sweat_smile: 
Thanks!
After rebasing #22236, I'm getting a failure related to this PR in the `circleci/py36_locale` job (https://circleci.com/gh/pandas-dev/pandas/18093):

```
=================================== FAILURES ===================================
___________________________ test_tick_add_sub[Nano] ____________________________

cls = <class 'pandas.tseries.offsets.Nano'>

    @pytest.mark.parametrize('cls', tick_classes)
>   @example(n=2, m=3)
    @example(n=800, m=300)
    @example(n=1000, m=5)
    @given(n=st.integers(-999, 999), m=st.integers(-999, 999))
    def test_tick_add_sub(cls, n, m):
E   hypothesis.errors.FailedHealthCheck: Data generation is extremely slow: Only produced 2 valid examples in 1.09 seconds (0 invalid ones and 0 exceeded maximum size). Try decreasing size of the data you're generating (with e.g.max_size or max_leaves parameters).
E   See https://hypothesis.readthedocs.io/en/latest/healthchecks.html for more information about this. If you want to disable just this health check, add HealthCheck.too_slow to the suppress_health_check settings for this test.

pandas/tests/tseries/offsets/test_ticks.py:40: FailedHealthCheck
---------------------------------- Hypothesis ----------------------------------
You can add @seed(183585162026757523236170973153487462479) to this test or run pytest with --hypothesis-seed=183585162026757523236170973153487462479 to reproduce this failure.
```
And again, this time in another test, but same build job (https://circleci.com/gh/pandas-dev/pandas/18150):

```
=================================== FAILURES ===================================
___________________________ test_tick_add_sub[Milli] ___________________________

cls = <class 'pandas.tseries.offsets.Milli'>

    @pytest.mark.parametrize('cls', tick_classes)
>   @example(n=2, m=3)
    @example(n=800, m=300)
    @example(n=1000, m=5)
    @given(n=st.integers(-999, 999), m=st.integers(-999, 999))
    def test_tick_add_sub(cls, n, m):
E   hypothesis.errors.FailedHealthCheck: Data generation is extremely slow: Only produced 2 valid examples in 1.14 seconds (0 invalid ones and 0 exceeded maximum size). Try decreasing size of the data you're generating (with e.g.max_size or max_leaves parameters).
E   See https://hypothesis.readthedocs.io/en/latest/healthchecks.html for more information about this. If you want to disable just this health check, add HealthCheck.too_slow to the suppress_health_check settings for this test.

pandas/tests/tseries/offsets/test_ticks.py:40: FailedHealthCheck
---------------------------------- Hypothesis ----------------------------------
You can add @seed(188896409658629737425924686489321258929) to this test or run pytest with --hypothesis-seed=188896409658629737425924686489321258929 to reproduce this failure.
```

I think this healthcheck is too restrictive. If the server is busy for that single second that the samples are generated, it might already be triggered... The error message already mentions how to disable it, maybe this should be considered as a follow-up...?
Ah, looks like a CI clock issue.  I'll open a follow-up PR in the next day with the following in conftest.py:

```python
from hypothesis import settings, HealthCheck

settings.default.suppress_health_check = (HealthCheck.TooSlow,)
                                       # HealthCheck.all() to disable all health checks
```