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

Map example from documentation does not type check with 6.43.2 #3296

Closed
jenshnielsen opened this issue Apr 17, 2022 · 12 comments
Closed

Map example from documentation does not type check with 6.43.2 #3296

jenshnielsen opened this issue Apr 17, 2022 · 12 comments
Labels
interop how to play nicely with other packages

Comments

@jenshnielsen
Copy link

jenshnielsen commented Apr 17, 2022

The following example taken from https://hypothesis.readthedocs.io/en/latest/data.html?highlight=lists#mapping
does no longer type check with 6.43.2 probably due to the changes in #3287

Running mypy 0.942 on (with python 3.9 on windows)

from hypothesis.strategies import lists, integers
lists(integers()).map(sorted).example()

results in

❯ mypy .\hypexample.py
hypexample.py:3: error: Argument 1 to "map" of "SearchStrategy" has incompatible type overloaded function; expected "Callable[[List[int]], List[SupportsRichComparisonT]]"
Found 1 error in 1 file (checked 1 source file)
@Zac-HD Zac-HD added the interop how to play nicely with other packages label Apr 17, 2022
@rsokl
Copy link
Contributor

rsokl commented Apr 17, 2022

I don't see how #3293 (which took precedent over #3287), which only changed the annotation of @given, could have affected the behavior of lists(integers()).map(sorted).example(). Indeed, if I install the previous version, the same issue occurs:

$ mypy --version
mypy 0.942
$ hypothesis --version
hypothesis, version 6.43.1

$ mypy ./hypexample.py
scratch.py:5: error: Argument 1 to "map" of "SearchStrategy" has incompatible type overloaded function; expected "Callable[[List[int]], List[SupportsRichComparisonT]]"
Found 1 error in 1 file (checked 1 source file)

pyright has no issue and infers the correct type list[int].

Can you point me to a combo of hypothesis/mypy versions where mypy runs clean on hypexample.py?

@Zac-HD
Copy link
Member

Zac-HD commented Apr 17, 2022

I reproduced this on Hypothesis 6.43.1, so as expected it's not #3287 - that only touched @given(), which isn't used here at all. I'm pretty sure this is actually a mypy bug:

from hypothesis.strategies import lists, integers

s = lists(integers())
reveal_type(s)
reveal_type(sorted)
s2 = s.map(sorted)
reveal_type(s2)

Output, reformatted for legibility:

$ mypy test.py 
test.py:4: note: Revealed type is "SearchStrategy[builtins.list[builtins.int*]]"
test.py:5: note: Revealed type is "Overload(
    def [SupportsRichComparisonT <: Union[_typeshed.SupportsDunderLT, _typeshed.SupportsDunderGT]] 
        (typing.Iterable[SupportsRichComparisonT`-1], *, key: None =, reverse: builtins.bool =) 
        -> builtins.list[SupportsRichComparisonT`-1], 
    def [_T] 
        (typing.Iterable[_T`-1], *, key: def (_T`-1) -> Union[_typeshed.SupportsDunderLT, _typeshed.SupportsDunderGT], reverse: builtins.bool =) 
        -> builtins.list[_T`-1]
    )"
test.py:6: error: Argument 1 to "map" of "SearchStrategy" has incompatible type overloaded function; expected
        "Callable[[List[int]], List[SupportsRichComparisonT]]"
test.py:7: note: Revealed type is "SearchStrategy[builtins.list*[SupportsRichComparisonT`-1]]"

So it seems that mypy is failing to see that list[int] satisfies typing.Iterable[SupportsRichComparisonT]. @sobolevn, any ideas?

@rsokl
Copy link
Contributor

rsokl commented Apr 17, 2022

SearchStrategy[builtins.list*[SupportsRichComparisonT-1]] -- the -1 seems pretty weird!

@Zac-HD
Copy link
Member

Zac-HD commented Apr 17, 2022

SearchStrategy[builtins.list*[SupportsRichComparisonT-1]] -- the -1 seems pretty weird!

The `-1 is how mypy represents "the first type bound to the given typevar here", i.e. "the thing that is int if you call this with a list[int]". Maybe not the most obvious repr, but it's a hard thing to represent in general...

@Zac-HD
Copy link
Member

Zac-HD commented Apr 17, 2022

Oh, and @jenshnielsen - thanks very much for reporting this! Even if it turns out not to be a bug in Hypothesis per se, you gave us a fantastic minimal repro which will help us to get it fixed somewhere 😄

@rsokl
Copy link
Contributor

rsokl commented Apr 17, 2022

The `-1 is how mypy represents "the first type bound to the given typevar here", i.e. "the thing that is int if you call this with a list[int]". Maybe not the most obvious repr, but it's a hard thing to represent in general...

Huh! 👨‍🎓

@jenshnielsen
Copy link
Author

jenshnielsen commented Apr 17, 2022

@Zac-HD @rsokl I think see now how this happens It's indeed a bit more complicated. That pr does indeed only touch given but the specific test where I discovered this https://github.com/QCoDeS/Qcodes/blob/master/qcodes/tests/drivers/test_lakeshore_325.py#L12 uses this combination (or something very similar) inside a given decorator. I bet that before this used to be considered untyped by mypy so that it was not type checked at all but with this change its now considered typed so the fact that mypy flags this construct now becomes clear.

@jenshnielsen
Copy link
Author

As in mypy will accept

from hypothesis import given
from hypothesis.strategies import lists, integers

@given(lists(integers()).map(sorted))
def test_foo(foo):
    assert isinstance(foo, list)

with hypothesis 6.43.1 but not with 6.43.2

@Zac-HD
Copy link
Member

Zac-HD commented Apr 17, 2022

Ah, yep, that makes sense. I'll check if pyright --strict is willing to accept SearchStrategy[Any]; if not I guess we'll have to regress on something and I'm inclined to roll back the recent change until mypy can fix their end 😢

@jenshnielsen
Copy link
Author

@Zac-HD Personally I would be perfectly happy to add a type ignore comment to my own project and use the latest hypothesis with this bug reported upstream to mypy

@rsokl
Copy link
Contributor

rsokl commented Apr 17, 2022

@Zac-HD SearchStrategy[Any] should be fine in pyright --strict. I was going to say that the use of Ex in @given doesn't really make sense.

In fact, I think we can use this as an opportunity to overload given to tell people that mixed pos/kwarg is forbidden:

@overload
def given(
    *_given_arguments: Union[SearchStrategy[Any], InferType],
) -> Callable[
    [Callable[..., Union[None, Coroutine[Any, Any, None]]]], Callable[..., None]
]:

@overload
def given(
    **_given_kwargs: Union[SearchStrategy[Any], InferType],
) -> Callable[
    [Callable[..., Union[None, Coroutine[Any, Any, None]]]], Callable[..., None]
]:

def given(
    *_given_arguments: Union[SearchStrategy[Any], InferType],
    **_given_kwargs: Union[SearchStrategy[Any], InferType],
) -> Callable[
    [Callable[..., Union[None, Coroutine[Any, Any, None]]]], Callable[..., None]
]:

here, using Ex in the overloads would be an error because it appears only once per overload.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 18, 2022

Since we've done what we can locally and opened an upstream issue, I'll close this PR. Once again,

thanks very much for reporting this! Even if it turns out not to be a bug in Hypothesis per se, you gave us a fantastic minimal repro which will help us to get it fixed somewhere 😄

@Zac-HD Zac-HD closed this as completed Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop how to play nicely with other packages
Projects
None yet
Development

No branches or pull requests

3 participants