Bump pyright to 1.1.367 and add type checking tests for pipeline API #9674
Conversation
github-actions
bot
added
relnotes-fix
Deploying pydantic-docs with
|
| Latest commit: |
733d345
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe362654.pydantic-docs.pages.dev |
| Branch Preview URL: | https://bump-pyright.pydantic-docs.pages.dev |
CodSpeed Performance ReportMerging #9674 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Thanks @adriangb!
Looks great overall, just had a few follow up questions about parametrized TypeAdapters and some of the # type: ignore statements in the test_pipeline.py file :).
| # this test works by adding type ignores and having pyright fail with | ||
| # an unused type ignore error if the type checking isn't working | ||
| Annotated[str, validate_as(int)] # type: ignore | ||
| Annotated[str, validate_as(str).transform(lambda x: int(x))] # type: ignore |
| @@ -25,12 +25,12 @@ | |||
|
|
|||
| @pytest.mark.parametrize('potato_variation', ['potato', ' potato ', ' potato', 'potato ', ' POTATO ', ' PoTatO ']) | |||
| def test_parse_str(potato_variation: str) -> None: | |||
| ta_lower = TypeAdapter(Annotated[str, validate_as(...).str_strip().str_lower()]) | |||
| ta_lower = TypeAdapter[str](Annotated[str, validate_as(...).str_strip().str_lower()]) | |||
There was a problem hiding this comment.
What error do we get if we don't explicitly parametrize the TypeAdapter? Is there any way for said type to be inferred from the annotation?
There was a problem hiding this comment.
Probably not haha I'm guessing you would have already handled that, but just wanted to check. Guessing it's the whole generic can of worms.
There was a problem hiding this comment.
Not until the TypeForm PEP gets merged...
I thought it used to end up as TypeAdapter[Any] but it seems like it doesn't now. Maybe because of 8a896c5?
There was a problem hiding this comment.
Looks like 953bbea#diff-2695629fb80cb6eb6be5260a4611b3b831a7ebd53041d01f37c0d67dd55928fa is the one that broke it
There was a problem hiding this comment.
Hum, in my #9570 fix I kept the __init__ so that users would be forced to parametrize the TypeAdapter if the type checker can't use the first overload. But having an implicit fallback to TypeAdapter[Any] seems good as well
tests/test_pipeline.py
Outdated
| @@ -67,18 +67,18 @@ def test_ge_le_gt_lt( | |||
| @pytest.mark.parametrize( | |||
| 'type_, pipeline, valid_cases, invalid_cases', | |||
| [ | |||
| (int, validate_as(int).multiple_of(5), [5, 20, 0], [18, 7]), | |||
| (float, validate_as(float).multiple_of(2.5), [2.5, 5.0, 7.5], [3.0, 1.1]), | |||
| (int, validate_as(int).multiple_of(5), [5, 20, 0], [18, 7]), # type: ignore | |||
There was a problem hiding this comment.
Why do we use an ignore here?
There was a problem hiding this comment.
It had to do with the typing of multiple_of. I'm still not 100% certain it's correct but since it's just typing good enough.
tests/test_pipeline.py
Outdated
| @@ -115,13 +115,13 @@ def test_interval_constraints(type_: Any, pipeline: Any, valid_cases: list[Any], | |||
| [ | |||
| ( | |||
| str, | |||
| validate_as(str).len(min_len=2, max_len=5), | |||
| validate_as(str).len(min_len=2, max_len=5), # type: ignore | |||
There was a problem hiding this comment.
Same q, why the ignore?
There was a problem hiding this comment.
This one was because some of the types like list where missing parametrization below (needs to be list[int])
Pyright now has provisional support for PEP 746: microsoft/pyright@ac7f6b7