-
-
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
[1.12] Regression in is None
narrowing
#18021
Comments
is None
narrowingis None
narrowing
This bisects to #17427. Here is a complete, slightly simplified reproducer: from typing import Literal
SEP = Literal["a", "b"]
class Base:
def feed_data(
self,
sep: SEP,
) -> int:
return 0
class C(Base):
def feed_data(
self,
sep: SEP | None = None,
) -> int:
if sep is None:
sep = reveal_type("a" if int() else "b") # Union[Literal['a'], Literal['b']?]
reveal_type(sep) # None
# Incompatible type "Literal['a', 'b'] | None"; expected "Literal['a', 'b']"
return super().feed_data(sep) Interestingly, we used to infer the correct union type before #17427, so this is perhaps not just a pre-existing issue that #17427 exposed but something more subtle. cc @ilevkivskyi |
Appears mypy_primer did catch it, but there were probably too many changes there for someone to notice the legitimate regression. |
Yeah. I think the vast majority of new errors were legitimate. |
Fixes #18021 When I switched to (almost) always using unions as inferred from ternary expressions, I had a choice, because before we used either full context (i.e. l.h.s.) or the if type to infer the else type. After some playing I found the second one usually works better. But as we see this is not always the case, so I add some special-casing.
Not sure this was fully resolved. The linked fix is in 1.14, but the aiohttp regression is still present: |
@Dreamsorcerer could you please find a simple self-contained repro? (I guess there may be something special about aiohttp setup) because the above example works for me in a recent master. |
@ilevkivskyi Sorry for the delay, been a slow month. I broke it down to a minimal reproducer, then realised that it's almost exactly the same as the above example. After some more testing I've realised that the reveal_type() is fixing the example. Simply removing the first reveal_type() reproduces the issue again:
|
OK, nice, I have a fix, but it is too late, I will make a PR tomorrow. |
@Dreamsorcerer here is the real fix #18545 |
Bug Report
Regression in 1.12.
I'm not sure if this is deliberate or somehow got missed, but I thought aiohttp was in the mypy test suite, so should have been caught in the regression tests..
https://github.com/aio-libs/aiohttp/blob/01bfb4380742df87f910c2c62c619e0e9b9a0ed4/aiohttp/http_parser.py#L678-L684
Now results in:
Expected Behavior
The variable can never be None at this point, so there should be no error (as in 1.11 and previous releases).
Your Environment
aio-libs/aiohttp#9527
The text was updated successfully, but these errors were encountered: