Skip to content
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

list.__add__ isn't permissive enough #8292

Closed
Akuli opened this issue Jul 13, 2022 · 3 comments · Fixed by #8293
Closed

list.__add__ isn't permissive enough #8292

Akuli opened this issue Jul 13, 2022 · 3 comments · Fixed by #8293

Comments

@Akuli
Copy link
Collaborator

Akuli commented Jul 13, 2022

Works at runtime, doesn't type check:

class Foo:
    def asd(self) -> int:
        return 1

class Bar:
    def asd(self) -> int:
        return 2

combined = [Foo()] + [Bar()]
for item in combined:
    print(item.asd())

I would expect [Foo()] + [Bar()] to produce list[Foo | Bar].

Converting to tuples works as expected:

combined = tuple([Foo()]) + tuple([Bar()])

But invariance problems don't apply here, because we're constructing a new list that doesn't refer to the old lists in any way.

There has previously been discussion about how mypy doesn't understand [Foo(), Bar()], but this is slightly different: I'm adding lists, not mixing types within different elements of a single list literal.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 13, 2022

It seems reasonable to change it to:

_T = TypeVar("_T")
_S = TypeVar("_S")

class list(MutableSequence[_T]):
    def __add__(self, __x: list[_S]) -> list[_T | _S]:...

We already do a similar thing with dict.__or__:

def __or__(self, __value: Mapping[_T1, _T2]) -> dict[_KT | _T1, _VT | _T2]: ...

@JelleZijlstra
Copy link
Member

This is trickier than it seems, see #3183 for the last attempt.

@Akuli
Copy link
Collaborator Author

Akuli commented Jul 13, 2022

I'll try again. The last attempt was done mostly before mypy_primer was a thing, so this time we should at least know if something breaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants