Add reusable type for @composite draw argument#3069
Add reusable type for @composite draw argument#3069Zac-HD merged 9 commits intoHypothesisWorks:masterfrom ROpdebee:add-composite-draw-arg-type-hint
@composite draw argument#3069Conversation
Zac-HD
left a comment
There was a problem hiding this comment.
Thanks Ruben - this looks good to me, and I really appreciate your detailed writeup!
- I agree with your analysis of why a
Callabletypevar is unsatisfactory. While I prefer not to add public API, it's clearly justified in this case. - IMO we should make it a
Protocolthough, at least on py38+ - see below. - I agree with your analysis of the interaction with PEP-612.
- This is a
minorrelease, independent of #3068 - we like to keep versions atomic. - I'd usually also suggest adding your name to the contributors list, but we can leave that for now to avoid merge conflicts.
It would be great to add a test for this to https://github.com/HypothesisWorks/hypothesis/blob/master/whole-repo-tests/test_type_hints.py, along the lines of "mypy runs on this code without errors"; after that I think we'll be ready to merge!
Zac-HD
left a comment
There was a problem hiding this comment.
Looks great! One tiny formatting issue, then we'll merge 🚀
|
@Zac-HD I just noticed that in the docs, the |
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
|
I've excluded the pre-3.8 initializer from coverage. It is covered by a test on Python versions where we use our own Also apologies for hammering CI and the force-pushes, for some reason the |
|
Hmm, flaky test? |
Fixes #3071.
|
Thanks again @ROpdebee! Now released as Hypothesis 6.16.0 🚀 |
I've split this off from #3068 to keep each PR self-contained and because this is perhaps a bit more of a controversial change. Also related to #3005.
In short, this change adds and exports a type,
DrawFnthat people using type annotations can use to annotate thedrawfunction provided by@composite. This goal of this new type is similar toDataObject, with one big difference being that this one exists solely for typing and doesn't serve any functional goals. Its main benefit is that it's reusable and a single point of definition of the interface that can be evolved if hypothesis evolves, which is much better than everyone copying over the suggested typevars from #3005 into their own files.If anyone can suggest a better name for this type, I'd be happy to change it!
A note on the implementation: I've implemented this as a class rather than a typevar (as suggested in #3005), since the
drawfunction seems to take an optional argumentlabeland it isn't possible to specify that with just atyping.Callablealone. Callback protocols do support this, buttyping.Protocolhas only landed in the stdlib since Python 3.8. Instead, this class acts like a protocol without actually inheriting from one. Its sole purpose is to describe something which, when called with aSearchStrategy[Ex]and an optional label, returns anEx. We never have to use this within hypothesis itself, it's purely a hint/tool for the user to use in their wrapped function annotation.A note for future maintenance: If you ever decide to make this an actual
typing.Protocol, make sure not to make the protocol itself generic, just the__call__method. So,class DrawFn(Protocol): ..., notclass DrawFn(Protocol[Ex]): .... The latter would require providing a type argument whenever theDrawFntype is used in an annotation (with strict mypy settings), the former does not.Also, although PEP 612 will allow for better typing of
@composite, I'm not sure whether type checkers would then be able to infer the parameter types of the decorated functions. The PEP provides an example regardingConcatenate, and there, the concatenated parameter (Request) is still annotated in the wrapped function. Moreover, strict settings for type checkers might continue to require annotations on each parameter, regardless of whether they can somehow infer the type. Therefore, I don't think this contribution would become obsolete once 3.10 becomes the minimum supported version.An example (adapted from the linked issue):
This type-checks correctly and without errors under the strictest mypy settings (excluding
--disallow-any-decorated).Similarly to #3068, I've pondered on the appropriate version bump type, and I feel like minor is more appropriate here than in #3068. If I may weigh in on the release strategy for these, I'd suggest releasing both PRs as a single release rather than each one individually.