-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[red-knot] Infer unary not operation for instances #13827
Conversation
5c94230
to
4586776
Compare
4586776
to
f970b04
Compare
|
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
5b2766b
to
e0ed3f7
Compare
1ce301c
to
bc90f1a
Compare
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Left a few comments. The good news is I think the comments should make the implementation much simpler.
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!!
Only minor wording nits and one other comment; I'll just apply these and look into the call_dunder
question briefly, and then land this.
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/unary/instance.md
Outdated
Show resolved
Hide resolved
Thanks for helping me why my initial approach was wrong. I will try to make it less work for you next time. |
600f49e
to
cb250e0
Compare
No problem, thank you for the contributions!! |
Also, to be clear, your initial approach was wrong mostly because we had a TODO comment in the source code specifically suggesting the wrong approach 😆 |
My attempt at resolving not op for instances.
The type is true unless one of the following happens:
__bool__
and if it is false then the result is false.Test Plan
I added a new test case with some examples from these resources: