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

Matchers should be contravariant #222

Open
mezuzza opened this issue Nov 27, 2022 · 7 comments
Open

Matchers should be contravariant #222

mezuzza opened this issue Nov 27, 2022 · 7 comments

Comments

@mezuzza
Copy link

mezuzza commented Nov 27, 2022

Currently, the Matcher class has a type arg T as it should, but Matchers are currently invariant with respect to T. However, I think it's quite clear that the following should be legal code:

matcher: Matcher[str] = has_length(greater_than(0))

I've recreated that definition here:

from typing import Any, Generic, Sequence, Sized, TypeVar

T = TypeVar("T")


class Matcher(Generic[T]):
  def matches(self, _: T) -> bool:
    ...


def greater_than(_: Any) -> Matcher[Any]:
  ...


def has_length(_: int | Matcher[int]) -> Matcher[Sized]:
  ...


matcher: Matcher[str] = has_length(greater_than(0))

Pyright provides the following error:

» pyright test.py
[...]
test.py
  test.py:23:25 - error: Expression of type "Matcher[Sized]" cannot be assigned to declared type "Matcher[str]"
    "Matcher[Sized]" is incompatible with "Matcher[str]"
      TypeVar "T@Matcher" is invariant
        "Sized" is incompatible with "str" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations
Completed in 0.582sec

Changing line 3 to T = TypeVar('T', contravariant=True) fixes the issue.

python version: 3.10.8
pyhamcrest version: 2.0.4
pyright version: 1.1.280

@brunns
Copy link
Member

brunns commented Nov 28, 2022

Thanks, I'll look into this in a few days if no one else gets to it first.

@mezuzza
Copy link
Author

mezuzza commented Nov 28, 2022

Thanks a lot. I think this should fix a number of type problems when using the library.

@brunns brunns self-assigned this Nov 28, 2022
@brunns
Copy link
Member

brunns commented Dec 8, 2022

Ah, I remember now. So, I did try this back when I added typing, but the trouble is getting the contravariant matcher to work with assert_that(). I was never able to work it out.

@mezuzza
Copy link
Author

mezuzza commented Dec 8, 2022

Maybe I'm missing something, but

from typing import Any, Generic, Sized, TypeVar

T = TypeVar("T", contravariant=True)


class Matcher(Generic[T]):
  def matches(self, _: T) -> bool:
    ...


def greater_than(_: Any) -> Matcher[Any]:
  ...


def has_length(_: int | Matcher[int]) -> Matcher[Sized]:
  ...


matcher: Matcher[str] = has_length(greater_than(0))


T2 = TypeVar("T2")


def assert_that(
  actual_or_assertion: T2, matcher: Matcher[T2], reason: str = ""
) -> None:
  ...


assert_that("hi there bob", has_length(greater_than(0)))

Doesn't give me any errors.

Was there a specific case that fails?

@brunns
Copy link
Member

brunns commented Dec 8, 2022

It may be be me who is missing something! Or perhaps the overrides are the problem? But in any case, I can't get this working in the real codebase.

Do you want to try it? I'm sure a PR would be gratefully received as long as it's backward compatible.

@mezuzza
Copy link
Author

mezuzza commented Dec 15, 2022

I can give it a shot later when I get some time. Probably won't be for a bit though.

@brunns
Copy link
Member

brunns commented Dec 15, 2022

Thanks, I appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants