-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add regression tests for inference bug repros #4387
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
Add regression tests for inference bug repros #4387
Conversation
Ref pylint-dev/pylint#4083. Ref pylint-dev/pylint#4387. When used in a nodes.Subscript, an existing inference_tip was set for Name nodes matching called type. However, when this name was redefined this led to false-positive typecheck errors in pylint such as invalid-sequence-index (see pylint-dev/pylint#4083 and pylint-dev/pylint#4387)
Ref pylint-dev/pylint#4083. Ref pylint-dev/pylint#4387. When used in a nodes.Subscript, an existing inference_tip was set for Name nodes matching called type. However, when this name was redefined this led to false-positive typecheck errors in pylint such as invalid-sequence-index (see pylint-dev/pylint#4083 and pylint-dev/pylint#4387)
Ref pylint-dev/pylint#4083. Ref pylint-dev/pylint#4387. When used in a nodes.Subscript, an existing inference_tip was set for Name nodes matching called type. However, when this name was redefined this led to false-positive typecheck errors in pylint such as invalid-sequence-index due to the argument being inferred as builtins.type and inference_tip preventing any further inference.
Ref pylint-dev/pylint#4083. Ref pylint-dev/pylint#4387. When used in a nodes.Subscript, an existing inference_tip was set for Name nodes matching called type. However, when this name was redefined this led to false-positive typecheck errors in pylint such as invalid-sequence-index due to the argument being inferred as builtins.type and inference_tip preventing any further inference.
a406b17
to
76dc276
Compare
I rebased on the latest main and opened #6521 to merge the commit that is passing right now. It look like the other two are still an issue. |
76dc276
to
750a4f8
Compare
@Pierre-Sassoulas At what point do we close these stale old PRs? It might make it easier to do management of the project if we keep only active PRs open? |
True, we can take over even if the PR is closed. I often don't dare to close to not offend the implementer. (My own MR often go stale) |
I have added "Needs take over" to most stale PRs. I think we can craft a friendly message explaining we're doing some clean up and then close most of them :) |
750a4f8
to
6184c76
Compare
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.
Let's merge this regression test before it's fixed, thank you Nelfin :)
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Steps
doc/whatsnew/<current release.rst>
.Description
Type of Changes
Related Issue