add types to hypothesis.internal.conjecture.utils

[adriangb]

1. Install monkeytype and the pytest plugin: pip install monkeytype pytest-monkeytype.
2. Export an env var to tell MonkeyType to only look in the module we want: export MONKEYTYPE_TRACE_MODULES=hypothesis.internal.conjecture
3. Run tests: pytest --monkeytype-output=./monkeytype.sqlite3 hypothesis-python/tests/conjecture
4. Apply some type annotations: monkeytype apply hypothesis.internal.conjecture.utils
5. Review and adjust

[Zac-HD]

Seems promising to me - I'm looking forward to seeing how this goes!

If it's useful, feel free to push incomplete code and tag me to clean up any tricky parts.

[Zac-HD]

(for the formatting, try pip install shed and then just shed to format all files in the repo)

[adriangb]

Thank you Zac. Tests are now passing. I see that we have 2 options here:

1. Go module by module. We merge this, then I open another PR for hypothesis.internal.conjecture.data, etc.
2. I make this PR a lot larger and try to do all modules in this PR.

I'm okay with either, but which version of this would you like / like to review?

[Zac-HD]

🎉

Since you're OK with it I'd prefer to take a single large PR; it's net-easier to review all at once, and avoids the risk of introducing annotations which we later find are inconsistent with another submodule. Thanks again for taking this on!

[adriangb]

FYI I pushed several more files and remote the history so that we'll have some ability to accept partial changes.

[adriangb]

@Zac-HD I think I need to go back on what I said above: as I've dug deeper, e.g. the shrinking subfolder I think this is too much for one PR. There are things that I'd like to discuss with you as we go along, e.g. places where I'm guessing we should use a TypeVar or a SupportsLT protocol but since there's only ever ints used in tests I'm not really sure.

I've also personally been quite busy and have some travel coming up; I don't want this to rot.

So if you're okay with it, I would rather discuss what's here so far (a somewhat arbitrary subset of the module) and then go from there.

[Zac-HD]

That sounds good to me; I'm also happy to take things incrementally and it's already getting pretty substantial. Quick thougts:

• We probably do only want literal int in a bunch of places; conjecture is a rather low-level chunk of code. (We have at various times considered porting it to Rust which could be shared by Python/Ruby/etc frontends, though I suspect MypyC will be an easier build to coordinate)
• What's your preference between "Zac gives a detailed review" and "Zac just pushes a commit with some nitpicks and then merges"? I'm happy either way.
• When we eventually get everything annotated, we should also turn up the strictness of the typecheckers on this submodule.
• Thanks again for contributing 😍

[adriangb]

I have no problem with you pushing to this branch and no problem with nit picks either. For some changes it will be easier to throw them into my court and for others it may be easier for you to just fix, I leave it to your discretion.

Either way I'll look at any changes you make and ask questions if they don't make sense to me since it'll be important to understand for any further changes.

[adriangb]

So this part I'm a bit confused about. Looking at ExampleProperty.finish the base class returns an IntList. But this subclass (starts_and_ends) returns a Tuple[IntList, IntList], which I think is a violation of the LSP and hence hard to type without ignoring/Any. Does this sound right or am I missing something?

[Zac-HD]

Yeah, sounds right, I'd guess we just have an expedient LSP-violating design there (and have to date gotten away with it because it's all private internals and we're just using inheritance for code sharing).

[Zac-HD]

Comments below, plus mypy failures in CI here.

Let me know if or when you'd like to hand it off for me to finish - which I'm genuinely happy to do; until then I'll leave it with you to avoid duplicate work.

[adriangb]

How would you like to deal with dataclass_transform?
Without it, MyPy will complain because all of the attrs classes are untyped.
But Hypothesis does not currently depend on typing_extensions.
We could make it required for testing, but not for users:

if TYPE_CHECKING:
    from typing_extensions import dataclass_transform
else:
    def dataclass_transform() -> Callable[[T], T]:
        def wrapper(cls: T) -> T:
            return T
    return wrapper

[Zac-HD]

I think "required for type-checking, not required for users" is the ideal situation.

That would mean listing it in tools.in, so that that the tests themselves also run without optional dependencies - otherwise it's all too plausible that we accidentally make it a hard dependency someday.

[Zac-HD]

🎉

Thanks again for your contribution, Adrian!

[adriangb]

Thank you for the guidance!