Switch type-checking from pyright to ty#242
Conversation
| # Conformance test files import generated code as `from gen.connectrpc...` and | ||
| # sibling modules like `_util`/`_cov_embed`. Pytest and the launched server/client | ||
| # subprocesses put `conformance/test/` on sys.path; teach ty to do the same. | ||
| extra-paths = ["conformance/test"] |
There was a problem hiding this comment.
this, plus dropping the __init__.py and reworking the imports above let us typecheck these.
stefanvanburen
force-pushed
the
svanburen/ty
branch
from
cce488a to
36e6a14
Compare
Figured I'd see how far along `ty` is; feels pretty good to me. Was able to eliminate more of the type checker ignores along the way. Signed-off-by: Stefan VanBuren <svanburen@buf.build>
stefanvanburen
force-pushed
the
svanburen/ty
branch
from
36e6a14 to
a2637d3
Compare
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
anuraaga
left a comment
anuraaga
left a comment
There was a problem hiding this comment.
One random side effect is using the default pyright version in VSCode for normal setups (not using ty language server which will continue to be the norm for a long time). Amazingly one patch version of drift (it loads 1.1.408) seems to cause this pyright bug in widening bytes to buffer protocol. Not our problem I would say but figured worth a share.
| "tombi==0.10.2", | ||
| "ty==0.0.34", | ||
| "types-grpcio==1.0.0.20260408", | ||
| "types-protobuf==7.34.1.20260503", |
There was a problem hiding this comment.
Thanks - didn't realize there was this hatch for getting protobuf working with it
| ) | ||
|
|
||
| if is_unary and isinstance(protocol, ConnectServerProtocol): | ||
| # Re-check via isinstance (rather than reusing `is_unary`) so the type |
There was a problem hiding this comment.
I couldn't reproduce the issue - pushed a change to revert it
There was a problem hiding this comment.
thanks, I thought I double-checked this; oh well!
| import pytest | ||
|
|
||
| from ._util import VERSION_CONFORMANCE, coverage_env, maybe_patch_args_with_debug | ||
| from _util import VERSION_CONFORMANCE, coverage_env, maybe_patch_args_with_debug |
There was a problem hiding this comment.
I guess this doesn't hurt though adding conformance to the search paths would also make sense, I guess any pytest root should be added to it in principle.
Reminds me of the days when conformance was a separate pyproject, with renovate working properly we may consider eventually going back to that to reduce pythonpath worries
| from asyncio import ( | ||
| timeout as asyncio_timeout, # pyright: ignore[reportAttributeAccessIssue] | ||
| ) | ||
| from asyncio import timeout as asyncio_timeout # ty: ignore[unresolved-import] |
There was a problem hiding this comment.
Using ImportError is a very old habit of mine for shims, but ever since version_info tuple I guess using that may be more idiomatic (typing-extensions uses it extensively), can consider rewriting to that to be properly type aware
Allows us to drop the ty ignore. Signed-off-by: Stefan VanBuren <svanburen@buf.build>
Redux of #418 - `ty` is still iterating, but now used over in connect-python (connectrpc/connect-python#242); seems reasonable to be consistent across our projects.
2 participants
Figured I'd see how far along
tyis; feels pretty good to me. Was able to eliminate more of the type checker ignores along the way.