Skip to content
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

Merged
merged 15 commits into from
Nov 13, 2024

Conversation

Glyphack
Copy link
Contributor

@Glyphack Glyphack commented Oct 19, 2024

My attempt at resolving not op for instances.

The type is true unless one of the following happens:

  1. We call the __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:

@Glyphack Glyphack force-pushed the class-instance-bool branch 2 times, most recently from 5c94230 to 4586776 Compare October 20, 2024 13:00
@Glyphack Glyphack changed the title [red-knot] WIP Handle class and instance cases for bool value [red-knot] Handle class and instance cases for bool value Oct 20, 2024
@Glyphack Glyphack force-pushed the class-instance-bool branch from 4586776 to f970b04 Compare October 20, 2024 13:04
Copy link
Contributor

github-actions bot commented Oct 20, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@Glyphack Glyphack force-pushed the class-instance-bool branch from 5b2766b to e0ed3f7 Compare November 9, 2024 18:27
@Glyphack Glyphack changed the title [red-knot] Handle class and instance cases for bool value [red-knot] Infer bool for class literal and instances Nov 9, 2024
@Glyphack Glyphack force-pushed the class-instance-bool branch from 1ce301c to bc90f1a Compare November 10, 2024 17:40
@Glyphack Glyphack changed the title [red-knot] Infer bool for class literal and instances [red-knot] Infer bool for instances Nov 10, 2024
@Glyphack Glyphack changed the title [red-knot] Infer bool for instances [red-knot] Infer unary not operation for instances Nov 10, 2024
Copy link
Contributor

@carljm carljm left a 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/src/types.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/infer.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/infer.rs Outdated Show resolved Hide resolved
crates/red_knot_python_semantic/src/types/infer.rs Outdated Show resolved Hide resolved
@Glyphack Glyphack marked this pull request as ready for review November 13, 2024 22:28
Copy link
Contributor

@carljm carljm left a 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.

@carljm carljm enabled auto-merge (squash) November 13, 2024 23:09
@Glyphack
Copy link
Contributor Author

Glyphack commented Nov 13, 2024

Thanks for helping me why my initial approach was wrong. I will try to make it less work for you next time.

@carljm carljm force-pushed the class-instance-bool branch from 600f49e to cb250e0 Compare November 13, 2024 23:26
@carljm
Copy link
Contributor

carljm commented Nov 13, 2024

Thanks for helping me why my initial approach was wrong. I will try to make it less work for you next time.

No problem, thank you for the contributions!!

@carljm carljm merged commit 924741c into astral-sh:main Nov 13, 2024
18 checks passed
@carljm
Copy link
Contributor

carljm commented Nov 13, 2024

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 😆

@Glyphack Glyphack deleted the class-instance-bool branch November 13, 2024 23:40
@carljm carljm added the red-knot Multi-file analysis & type inference label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants