False positive HypothesisDeprecationWarning

[Tinche]

Just upgraded to Hypothesis 3.8.2, and I'm getting the following warning:

.venv/lib/python3.6/site-packages/hypothesis/strategies.py:416: HypothesisDeprecationWarning: Cannot sample from <enum 'AnEnum'>, not a sequence.  Hypothesis goes to some length to ensure that sampling an element from a collection (with `sampled_from` or `choices`) is replayable and can be minimised.  To replay a saved example, the sampled values must have the same iteration order on every run - ruling out sets, dicts, etc due to hash randomisation.  Most cases can simply use `sorted(values)`, but mixed types or special values such as math.nan require careful handling - and note that when simplifying an example, Hypothesis treats earlier values as simpler.
  elements = check_sample(elements)

It's a great warning but I think I'm getting a false positive:

from hypothesis import given
from hypothesis.strategies import sampled_from

from enum import Enum


class AnEnum(Enum):
    A = "A"
    B = "B"


@given(sampled_from(AnEnum))
def test_enum(e):
    print(e)

According to https://docs.python.org/3/library/enum.html, "Enumerations support iteration, in definition order", so this should be fine, right?

[DRMacIver]

👍 We should definitely support enums here. Thanks for the report.

[Tinche]

It seems enums are Iterables, but not Sequences. Apparently for something to be a sequence it needs to support "efficient element access using integer indices via the __getitem__() special method" and enums don't do that, so yeah technically they aren't sequences.

Also dicts and hashes should be okay for 3.6+ and PyPy?

[DRMacIver]

Also dicts and hashes should be okay for 3.6+ and PyPy?

Hmm. That's true actually, yeah.

I guess we need a much more refined check for stable ordering.

[DRMacIver]

The ironic thing is that I spent quite a lot of time telling @Zac-HD we shouldn't be clever about this. 😜

I think being clever about certain concrete collection types might be OK though.

[DRMacIver]

As a side note, we introduced this deprecation in (I think) 3.7.1, which was naughty of us. We shouldn't introduce deprecation warnings in patch releases.

ETA: I say "us" I really mean "me" here. But now we're on one release per change it becomes us in future.

[alexwlchan]

As a side note, we introduced this deprecation in (I think) 3.7.1, which was naughty of us. We shouldn't introduce deprecation warnings in patch releases.

We definitely shouldn't introduce them without a changelog entry (at least, not one that I can see).

[DRMacIver]

We definitely shouldn't introduce them without a changelog entry (at least, not one that I can see).

Yeah, mea culpa. I did a very bad job of assembling the changelog entry for 3.7.1. But that problem has been fixed by the new release process, while deprecations in patch releases is a thing we've not yet pinned down a fix for (something to add to the review guide).

[Zac-HD]

The main idea was to only support things where the iteration order is stable between runs. Enums definitely count, but dict iteration order is still implementation-dependent.

I can add that pretty easily, but... @DRMacIver, I already have four patches in the queue waiting for review, and dealing with merge issues in the changelog is pretty tedious when I rebase and then go back to waiting. I've just added this to the tiny fix in #620, but do you mind reviewing that, #613, and #508 so I can integrate them?