-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Regression: 0.470->0.501] Lots of new errors relating to "uninhabited" in Django-based app code #2978
Comments
Yeah, I can repro this. @ddfisher The script uses I experimented with that one location a bit, and found:
Definitely sounds like a binder issue... |
@inducer If you want to help, maybe you can use |
Would love to, but paper deadline and 12 proposals to review over the next 7 days... Sorry! |
Under mypy 0.47, I still get:
|
Looks like some of the errors were caused by 9496d6f; I'm still trying to locate the others. |
Oof, this one's a mess...more of the errors were from 9406886, but there are still some that haven't come up yet... |
Around half of the errors were from, bizarrely enough, 3cc5201. So, to sum it up:
|
Also, I can confirm that 9496d6f was the root cause; temporarily reverting it made the errors go away. |
Thanks for the great investigation! Hopefully Jukka knows how to fix it. |
Reduced test case: # Run mypy with --strict-optional
from typing import Any, Optional
class A(Any): pass
def f(a: Optional[A]):
if a is not None:
reveal_type(a) (The Ironically enough, the bad commit's message says:
:/ |
Also, this was caused by the same commit: from typing import Any, Union
class A(Any): pass
class B: pass
def f(a: Union[A, B]):
if not isinstance(a, B):
reveal_type(a) # Union[<ERROR>, <ERROR>] |
Oh well, I knew that the bad commit was only a partial fix, but I didn't foresee these problems. A more complete fix that I was working on caused many problems with internal Dropbox code. The bad commit fixed other urgent issues so just reverting it is not a good option. I'll look at finishing up the more complete fix. |
Well, one problem is we need to kill this: if any(isinstance(typ, AnyType) for typ in items):
return AnyType() |
@ddfisher Yup, that's gone in my private branch. |
The main change is that unions containing Any are no longer simplified to just Any. Also union simplification now has a deterministic result unlike previously, when result could depend on the order of items in a union (this is true modulo remaining bugs). This required changes in various other places to keep the existing semantics, and resulted in some fixes to existing test cases. I also had to fix some tangentially related minor bugs that were triggered by the other changes. We generally don't have fully constructed TypeInfos so we can't do proper union simplification during semantic analysis. Just implement simple-minded simplification that deals with the cases we care about. Fixes #2978. Fixes #1914.
The main change is that unions containing Any are no longer simplified to just Any. Also union simplification now has a deterministic result unlike previously, when result could depend on the order of items in a union (this is true modulo remaining bugs). This required changes in various other places to keep the existing semantics, and resulted in some fixes to existing test cases. I also had to fix some tangentially related minor bugs that were triggered by the other changes. We generally don't have fully constructed TypeInfos so we can't do proper union simplification during semantic analysis. Just implement simple-minded simplification that deals with the cases we care about. Fixes #2978. Fixes #1914.
I think that this is still open because the fix was reverted. |
used to work with no warnings in 0.470. Now, it spews this:
Focusing on the first error:
https://github.com/inducer/relate/blob/fed97182f4e8940bb032a224c5cbcd007a8bd722/course/content.py#L877
it should be clear that
course
is an instance ofCourse
(i.e. not None).The text was updated successfully, but these errors were encountered: