-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
[WIP] Fix crash when joining tuple and NamedTuple #3129
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
Changes from all commits
a72c71b
2e90e9f
936bbe6
5810f55
bc631d9
865ec24
a255694
a27613a
84b8fae
ea2b1b4
a230dd9
02961f4
a993ba6
fb4ce35
f292ea4
52b0c0f
07e7e64
f991a2b
c1304ba
959e6c9
8edde23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -425,6 +425,7 @@ class Instance(Type): | |
|
||
def __init__(self, typ: mypy.nodes.TypeInfo, args: List[Type], | ||
line: int = -1, column: int = -1, erased: bool = False) -> None: | ||
# TODO: assert(typ is NOT_READY or typ.fullname() != 'builtins.tuple' or len(args) == 1) | ||
assert(typ is NOT_READY or typ.fullname() not in ["builtins.Any", "typing.Any"]) | ||
self.type = typ | ||
self.args = args | ||
|
@@ -888,6 +889,8 @@ def __init__(self, items: List[Type], fallback: Instance, line: int = -1, | |
column: int = -1, implicit: bool = False) -> None: | ||
self.items = items | ||
self.fallback = fallback | ||
# TODO: assert not (isinstance(fallback, Instance) and fallback.type and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to see the TODO items resolved before this is merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't looked into this yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And again, not sure why assert fails. I'll commit fixes to the other problems first. |
||
# fallback.type.fullname() == 'builtins.tuple' and not fallback.args) | ||
self.implicit = implicit | ||
self.can_be_true = len(self.items) > 0 | ||
self.can_be_false = len(self.items) == 0 | ||
|
@@ -922,7 +925,10 @@ def copy_modified(self, *, fallback: Optional[Instance] = None, | |
return TupleType(items, fallback, self.line, self.column) | ||
|
||
def slice(self, begin: int, stride: int, end: int) -> 'TupleType': | ||
return TupleType(self.items[begin:end:stride], self.fallback, | ||
new_items = self.items[begin:end:stride] | ||
fallback_args = [UnionType.make_simplified_union(new_items)] | ||
new_fallback = self.fallback.copy_modified(args=fallback_args) | ||
return TupleType(new_items, new_fallback, | ||
self.line, self.column, self.implicit) | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,7 +91,7 @@ from typing import Tuple | |
t1 = None # type: Tuple[A, A] | ||
t2 = None # type: tuple | ||
|
||
t1 = t2 # E: Incompatible types in assignment (expression has type Tuple[Any, ...], variable has type "Tuple[A, A]") | ||
t1 = t2 # E: Incompatible types in assignment (expression has type tuple, variable has type "Tuple[A, A]") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the test would fail then -- or did you mean to change the code the generates error messages? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a change, but for consistency, also changed a few other similar cases (which involve types other than |
||
t2 = t1 | ||
|
||
class A: pass | ||
|
@@ -604,10 +604,10 @@ b = s in t | |
|
||
[file builtins.py] | ||
from typing import TypeVar, Generic | ||
_T = TypeVar('_T') | ||
_T_co = TypeVar('_T_co', covariant=True) | ||
class object: | ||
def __init__(self) -> None: pass | ||
class tuple(Generic[_T]): | ||
class tuple(Generic[_T_co]): | ||
def __len__(self) -> int: pass | ||
def __str__(self) -> str: pass | ||
def __contains__(self, o: object) -> bool: pass | ||
|
@@ -702,6 +702,7 @@ B()[100] | |
[case testValidTupleBaseClass] | ||
from typing import Tuple | ||
class A(tuple): pass | ||
[builtins fixtures/tuple.pyi] | ||
[out] | ||
|
||
[case testTupleBaseClass2-skip] | ||
|
@@ -913,7 +914,7 @@ def f(a: Tuple) -> None: pass | |
f(()) | ||
f((1,)) | ||
f(('', '')) | ||
f(0) # E: Argument 1 to "f" has incompatible type "int"; expected Tuple[Any, ...] | ||
f(0) # E: Argument 1 to "f" has incompatible type "int"; expected tuple | ||
[builtins fixtures/tuple.pyi] | ||
|
||
[case testTupleSingleton] | ||
|
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.
Why this is still a TODO? I thought the main idea is to fix this specific point.
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.
Because it seemed to pass in the real world, but fail in tests. Let me figure out what stubs it needs... Or maybe I'll just sneak in some no-fixture tests into the test suite while nobody's watching.
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.
:-) I like the idea of running test with real full stubs, but let us postpone this for some time.
It is better to update fixtures for now, to make them match real stubs more.
Actually, the fact that some stubs can crash here is a bit dangerous, maybe we are still missing something?
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.
The problem with
Awaitable
is fixed, the problem withNewType
I'll wait for you to fix it in your PR as we discussed. There may be other problems that I haven't investigated yet.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.
Hmm still not sure why the assert fails, I'll look into it later.