[red-knot] Implement Type::Tuple Comparisons#13712
Conversation
|
carljm
left a comment
There was a problem hiding this comment.
This is really excellent, thank you!! A few comments.
884362d to
860a8e6
Compare
cbf3c44 to
3dbdae8
Compare
|
In addition to the changes mentioned in your review, I have enhanced the inference for |
AlexWaygood
left a comment
There was a problem hiding this comment.
Thanks! Some comments below:
crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md
Outdated
Show resolved
Hide resolved
AlexWaygood
left a comment
There was a problem hiding this comment.
Thanks! This LGTM now, but I'd love a second approval from either @carljm or @dhruvmanila
carljm
left a comment
There was a problem hiding this comment.
This looks good to me, modulo a few minor comment nits. Thanks for your work here!
I would like better test coverage (fewer todos). For example, right now the "unsupported comparison" case isn't really covered by the tests, because we don't have any rich-comparison operators implemented to return None from infer_binary_type_comparison for unsupported comparisons yet.
But the code here looks correct, so I'm happy to merge it as-is; the effective test coverage here will improve as we flesh out infer_binary_type_comparison.
Summary
This PR implements comparisons for (tuple, tuple).
It will close #13688 and complete an item in #13618 once merged.
Test Plan
Basic tests are included for (tuple, tuple) comparisons.