Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add static typing to Hypothesis API #200

Closed
DRMacIver opened this issue Oct 14, 2015 · 21 comments · Fixed by #1253 or #1270
Closed

Add static typing to Hypothesis API #200

DRMacIver opened this issue Oct 14, 2015 · 21 comments · Fixed by #1253 or #1270
Assignees
Labels
enhancement it's not broken, but we want it to be better meta for wider topics than the software itself

Comments

@DRMacIver
Copy link
Member

The new typing module in 3.5 and PEP 0484 add static type annotations to Python. It would be nice for the Hypothesis API to support this, as the public API is actually pretty static in nature. stub files offer a compatibility mode, so this can be done transparently without dropping support for Python 2 (which is sadly there for the long haul).

Steps this would require:

  1. Making Strategy a generic type with a single parameter, Strategy[T] that specifies the type of value generated. Note that Strategy is covariant in its type parameter.
  2. Providing static types for all of the public API in hypothesis and hypothesis.strategies.
  3. Running a mypy checker on Hypothesis + its test suite as part of the build

Things to note:

  1. given and builds cannot be precisely typed with this type system, but they can still have types that are better than nothing by constraining their *args and **kwargs to have a strategy type (this is actually quite useful because it will catch two common gotchas when using this).
  2. Settings is going to be quite hard to type correctly and is probably not worth bothering with.
  3. I am not proposing to make Hypothesis internals typed. In particular templates for strategies will remain completely untyped. I've worked out what typing this accurately requires in the past and PEP 0484 can't begin to express it.
@DRMacIver DRMacIver added enhancement it's not broken, but we want it to be better help-wanted labels Oct 14, 2015
@DRMacIver
Copy link
Member Author

Note: On investigation, I don't think running mypy as part of the Hypothesis build is currently feasible, and I'm not very interested in adding this without a static checking layer as part of the build. I will leave this open, but this should be considered a long-term objective rather than something viable right now.

@Zac-HD
Copy link
Member

Zac-HD commented Aug 23, 2017

Another note: the comments above were written about Hypothesis v1.11, and Hypothesis 3.x is going to be much nicer to annotate. MyPy has also improved substantially.

That said, we'd still have to use stub (.pyi) files, so I'm not volunteering to add type hints before we drop Python 2 at EOL in 2020. My work on #643 also led to a terrible cynicism about the typing module, which is shockingly inconsistent between 3.5, 3.6, and even patch versions.

@Zac-HD
Copy link
Member

Zac-HD commented Sep 13, 2017

It seems that this issue so far has been planning to type-annotate as much as possible. However, one of the main points of gradual typing is that you can add it to a project incrementally!

This post about Zulip has some good advice, which - summarised for Hypothesis - suggests:

  • Writing a make task that runs the smallest possible set of checks, getting a clean run, adding it to .travis.yml. This is 'glorified linter' territory, and shouldn't require code changes beyond (maybe) annotating empty global variables.
  • Adding the --check-untyped-defs option makes mypy try to infer variable types anywhere it can, without annotations. With luck, 'independently useful linter' territory.
  • Then anyone can add annotations (in comments or stubs, for Py2 compatibility) to various parts of the codebase in future PRs. This is where type hints become really useful and mypy does substantially more than any other Python linters.

@DRMacIver, if you think steps one and two are useful in isolation I'd be happy to make an attempt, and from there it's mostly an incremental job.

The remaining question is basically whether to use comments or stub files (until we drop Python2 support, at which point we can use inline annotations). Comments are more local to the related code, but if we use stub files it would be possible to exclude them from the has_source_changes check and therefore avoid releases for hint-only PRs.

@Daenyth
Copy link

Daenyth commented Sep 13, 2017 via email

@Zac-HD
Copy link
Member

Zac-HD commented Sep 13, 2017

Based on the PEP and typeshed docs, I thought that either comments or stubs in the Hypothesis package (ie in local code, not typeshed) would work - assuming that people use the --follow -imports option, obviously. I think this makes the stubs/comments distinction moot for downstream type-checking.

@Daenyth
Copy link

Daenyth commented Sep 13, 2017

Currently either comments or stubs will work for local checking within the hypothesis project. It won't work for people pip installing hypothesis.
See python/typing#84

@DRMacIver
Copy link
Member Author

DRMacIver commented Sep 13, 2017

I've gotta be honest, if the process of doing this correctly requires submitting pull requests to typeshed, my inclination is not to bother and to wait for the ecosystem around this to mature into something actually useful.

@Daenyth
Copy link

Daenyth commented Sep 13, 2017

I can't argue with that. The work involved is silly, though there's a pep in the progress of being drafted to address the issue.

I think it's fairly reasonable to also just have them locally inline (with comments) and wait for the ecosystem to allow pip-install to provide them, ignoring typeshed and downstream consumers.

@DRMacIver
Copy link
Member Author

I think it's fairly reasonable to also just have them locally inline (with comments) and wait for the ecosystem to allow pip-install to provide them, ignoring typeshed and downstream consumers.

That's true. I'm definitely not against that solution, and if @Zac-HD (or anyone else!) wants to put in the work to start doing that incrementally as he proposed, I'd be perfectly happy with that.

I'm personally still a bit sceptical about the typing module, but I don't have any problem with us giving it a go.

@Zac-HD
Copy link
Member

Zac-HD commented Sep 13, 2017

Whew, that was a longish read. My interpretation of the various issues and PEP 561 is that putting foo.pyi next to each foo.py will work for us now, and for downstream users once tooling improves. Until then, distributing type hints is simply more trouble than it's worth.

@Zac-HD Zac-HD added meta for wider topics than the software itself and removed help wanted labels Feb 20, 2018
@Zac-HD
Copy link
Member

Zac-HD commented Apr 14, 2018

Tooling has improved! Mypy now supports distributing type hints as described in PEP561 (docs). Options for discussion:

  1. provide stubs for downstream users, but don't run Mypy against Hypothesis
    • lightweight, but easy for implementation and stubs to get out of sync
  2. annotate Hypothesis API but avoid typing internals or defining generics
    • could use stubs or type comments (or annotations in 2020 once 3.4 and 2.7 die)
    • get some consistency checks from running Mypy in CI
  3. [start to] annotate everything

My preference is for option two:

  • aim for enough "gradual typing" to error if you pass the wrong type or number of arguments to public API or selected internals
  • don't bother (eg) defining SearchStrategy as a generic type, or typing @given beyond "pass it strategies"
  • ship as comments rather than stubs, to stay in sync (harder to ignore, for good or ill)

@DRMacIver
Copy link
Member Author

My preference is also for 2. Is there a reason you don't want SearchStrategy to be a generic type though? I feel like type checking the API will work much better if it is.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 14, 2018

Making SearchStrategy and @given generic would be nice, but may be an impractically large project and (without a clear mandate and excited users) I have other priorities.

I think I also have a higher opinion of 'shallow' typing than you! Especially given the error messages you're likely to see from typical code... For example, last time I tried Mypy on Hypothesis I found the dead classpath module, and this time I realised that Verbosity was trying to be an Enum (fixed in #1211).

TLDR: do generics later.

@DRMacIver
Copy link
Member Author

I think I also have a higher opinion of 'shallow' typing than you!

Well it would be hard to have a lower one. ;-)

TLDR: do generics later.

I don't think "do generics later" is actually a viable plan. The history of attempts at it is quite unpleasant. What happens if e.g. someone wants to use SearchStrategy in their own type signatures? (OK technically this isn't even public API, but it should probably be treated implicitly as such)

@Daenyth
Copy link

Daenyth commented Apr 14, 2018

As a user I'd be pretty excited by generic SearchStrategy. In fact, that's the only thing I care about being typed. The rest is so trivial it hardly helps, but generic SearchStrategy, especially with draw and flatmap is huge.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 15, 2018

  • Works on Python2
  • Works for Mypy
  • Annotations available for runtime inspection (builds, Sphinx, etc)

Wouldn't it be nice if we could have all three of these? Well, all we'd need to do is a spot of runtime introspection to translate either stubs or comments into an __annotations__ attribute for each callable in the public API 😄

This idea brought to you by sphinx-doc/sphinx#2755 and sphinx-doc/sphinx#4824, and thinking "hey, that could actually work..." (and "what could possibly go wrong?").

@Zac-HD
Copy link
Member

Zac-HD commented Apr 15, 2018

OK, OK, if you want generics up front we can do that.

(Fair warning: I will be very unhappy if we do this again, so please give frequent and detailed feedback)

What happens if e.g. someone wants to use SearchStrategy in their own type signatures? (OK technically this isn't even public API, but it should probably be treated implicitly as such)

We could insert a parent class Strategy with the same structure for use in type annotations, which just raises 'use the strategy functions' errors if you try to implement anything based on it.

@DRMacIver
Copy link
Member Author

(Fair warning: I will be very unhappy if we do this again, so please give frequent and detailed feedback)

I also want to avoid that! But I think the easiest way to make sure there is frequent and detailed feedback is to try to do the work in smaller chunks which can be shipped independently.

May I recommend an initial PR that does just enough work to make SearchStrategy generic and type check something in our build, but doesn't try to annotate the whole public API.

We could insert a parent class Strategy with the same structure for use in type annotations, which just raises 'use the strategy functions' errors if you try to implement anything based on it.

Is this a solution to the lack of it being in the public API or to it being non-generic? If the former we should probably resolve it as part of the renaming thing (both #804 and #1193)

@Zac-HD
Copy link
Member

Zac-HD commented Apr 15, 2018

Fully on board with shipping in small pieces.

How would you feel about a patch that just sets everything up for the real work in a future release? ie sort out build, packaging, get a clean Mypy run in Travis, etc; I've got most of that in a branch ready to go - or at least review.

@DRMacIver
Copy link
Member Author

how would you feel about a patch that just sets everything up for the real work in a future release?

Fine by me. I don't really mind either way - I'm not at all attached to this feature, so I'm mostly interested in making sure this goes smoothly for everyone than I am in any particular timeline.

@Zac-HD
Copy link
Member

Zac-HD commented May 9, 2018

Whoops, this should stay open until the final PR that distributes type hints!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement it's not broken, but we want it to be better meta for wider topics than the software itself
Projects
None yet
3 participants