Fix typing issues around Range/RangeValue #1196
Conversation
asyncpg/types.py
Outdated
| @@ -63,10 +63,10 @@ class _RangeValue(typing.Protocol): | |||
| def __eq__(self, __value: object) -> bool: | |||
| ... | |||
|
|
|||
| def __lt__(self, __other: _RangeValue) -> bool: | |||
| def __lt__(self: _RV, __other: _RV) -> bool: | |||
There was a problem hiding this comment.
I believe def __lt__(self, __other: Self) -> bool: should work
There was a problem hiding this comment.
Unfortunately it does not- I tried that first and settled on what was submitted to get it all working. Here's the output with your suggestion (made to both __lt__ and __gt__):
% pyright tests/test_types.py
/Users/dmcgee/projects/sbl/asyncpg/tests/test_types.py
/Users/dmcgee/projects/sbl/asyncpg/tests/test_types.py:18:25 - error: Argument of type "Literal[1]" cannot be assigned to parameter "lower" of type "_RV@Range | None" in function "__init__"
Type "Literal[1]" is not assignable to type "_RV@Range | None"
Type "Literal[1]" is not assignable to type "_RangeValue"
"Literal[1]" is incompatible with protocol "_RangeValue"
"__lt__" is an incompatible type
Type "(value: int, /) -> bool" is not assignable to type "(__other: _RV@__lt__, /) -> bool"
"__gt__" is an incompatible type
Type "(value: int, /) -> bool" is not assignable to type "(__other: _RV@__gt__, /) -> bool"
"Literal[1]" is not assignable to "None" (reportArgumentType)
There was a problem hiding this comment.
| def __lt__(self: _RV, __other: _RV) -> bool: | |
| def __lt__(self, __other: Self, /) -> bool: |
This will fix it, checked locally with pyright as well.
See typeshed which has the definition of int.__lt__ that pyright (correctly) consider to not be compatible with _RangeValue.__lt__:
https://github.com/python/typeshed/blob/ca65e087f1f9dfc28e89192b60ce7cfc2e92c674/stdlib/builtins.pyi#L321
asyncpg/types.py
Outdated
| @@ -63,10 +63,10 @@ class _RangeValue(typing.Protocol): | |||
| def __eq__(self, __value: object) -> bool: | |||
| ... | |||
|
|
|||
| def __lt__(self, __other: _RangeValue) -> bool: | |||
| def __lt__(self: _RV, __other: _RV) -> bool: | |||
There was a problem hiding this comment.
| def __lt__(self: _RV, __other: _RV) -> bool: | |
| def __lt__(self, __other: Self, /) -> bool: |
This will fix it, checked locally with pyright as well.
See typeshed which has the definition of int.__lt__ that pyright (correctly) consider to not be compatible with _RangeValue.__lt__:
https://github.com/python/typeshed/blob/ca65e087f1f9dfc28e89192b60ce7cfc2e92c674/stdlib/builtins.pyi#L321
In order for the `_RangeValue` protocol to type check properly, it needs to reference `Self`, not itself. Both pyright and mypy show a ton of errors when type checking this before these changes. (I've omitted non-relevant errors here): ``` % pyright tests/test_types.py /Users/dmcgee/projects/sbl/asyncpg/tests/test_types.py /Users/dmcgee/projects/sbl/asyncpg/tests/test_types.py:18:25 - error: Argument of type "Literal[1]" cannot be assigned to parameter "lower" of type "_RV@Range | None" in function "__init__" Type "Literal[1]" is not assignable to type "_RV@Range | None" Type "Literal[1]" is not assignable to type "_RangeValue" "Literal[1]" is incompatible with protocol "_RangeValue" "__lt__" is an incompatible type Type "(value: int, /) -> bool" is not assignable to type "(__other: _RV@__lt__, /) -> bool" "__gt__" is an incompatible type Type "(value: int, /) -> bool" is not assignable to type "(__other: _RV@__gt__, /) -> bool" "Literal[1]" is not assignable to "None" (reportArgumentType) /Users/dmcgee/projects/sbl/asyncpg/tests/test_types.py:18:34 - error: Argument of type "Literal[5]" cannot be assigned to parameter "upper" of type "_RV@Range | None" in function "__init__" Type "Literal[5]" is not assignable to type "_RV@Range | None" Type "Literal[5]" is not assignable to type "_RangeValue" "Literal[5]" is incompatible with protocol "_RangeValue" "__lt__" is an incompatible type Type "(value: int, /) -> bool" is not assignable to type "(__other: _RV@__lt__, /) -> bool" "__gt__" is an incompatible type Type "(value: int, /) -> bool" is not assignable to type "(__other: _RV@__gt__, /) -> bool" "Literal[5]" is not assignable to "None" (reportArgumentType) ... % mypy tests/test_types.py | grep arg-type tests/test_types.py:18: error: Argument "lower" to "Range" has incompatible type "int"; expected "None" [arg-type] tests/test_types.py:18: error: Argument "upper" to "Range" has incompatible type "int"; expected "None" [arg-type] ... (19 more errors) ``` After this change, the type checking comes back clean: ``` % pyright tests/test_types.py 0 errors, 0 warnings, 0 informations % mypy tests/test_types.py | grep arg-type <no output> ```
toofishes
force-pushed
the
fix-range-typing
branch
from
e00885a to
80502bf
Compare
|
Great suggestion - I made the update. I also verified that this works for |
|
Is there anything else I can do here to get this merged? |
|
Maybe @elprans can help review this? There are a couple other PRs from me related to typing that are also waiting on a review to get merged but I don't want to ping them too much. |
|
@elprans Sorry to ping again, but would it be possible to get a review on this and the other open |
Right on. Sorry for the review delays. |
|
Awesome! Appreciated! |
I'm glad to see some type hints were added in 0.30.0! I tried them out, and encountered a minor issue.
In order for the
_RangeValueprotocol to type check properly, it needs to reference the createdTypeVar, not itself.Both pyright and mypy show a ton of errors when type checking this before these changes. (I've omitted non-relevant errors here):
After this change, the type checking comes back clean:
This slight change appears to match what was expected when
bound=was implemented: python/typing#59 (comment)