-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[not-an-iterable] Add regression test for uninferable instances #9264
Conversation
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.
Are we inferring successfully that something is Iterable from a module we can't import because the namekf the class is 'Iterable' ? This would fail on deceiving code, right ? (but on the other hand it's probably working very well on Real World code)
From the comment, I took the motivation for the test to be testing an Uninferable. We weren't getting that. We were getting |
Oh! The bases being able to be inferred actually does matter. I'll just keep both tests (and improve the comment).`` |
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 investigating π
14165b0
to
fc83c51
Compare
Rebased for the docs check fix |
Nice github enhancement I just noticed: rebase + force push don't trigger the need for a new approving review ! |
Type of Changes
Description
While investigating #9253, I found that the regression test for suppressing
not-an-iterable
for Uninferable instances was not testing what it intended. Inference had improved over eight years, and we were testing anInstance
instead, notUninferable
.Refs #9253