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 concatenation problems with type context and different operand item types #5492

Closed
whoiscc opened this issue Aug 17, 2018 · 9 comments
Closed
Labels

Comments

@whoiscc
Copy link

whoiscc commented Aug 17, 2018

Here is a reproduction snippet:

from typing import List, Sequence, cast


class A:
    pass


class B1(A):
    pass


class B2(A):
    pass


# ok
list1: List[A] = [A()] * 10
# ok
list2: List[A] = [B1()] * 10
list2 += [B2()] * 10
# error!
list3: List[A] = [B1()] * 10 + [B2()] * 10
# error!
seq3: Sequence[A] = [B1()] * 10 + [B2()] * 10

# ok, but looks weird
list4: List[A] = [cast(A, B1())] * 10 + [B2()] * 10

Error report looks like this:

test.py:22: error: Incompatible types in assignment (expression has type "List[B1]", variable has type "List[A]")
test.py:22: error: List item 0 has incompatible type "B2"; expected "B1"
test.py:24: error: List item 0 has incompatible type "B2"; expected "B1"

The covariance of Sequence helps eliminate the first error. But I can never add two lists of objects of different classes without existing annotation, even the classes have same base.

In my opinion, cast takes place when downcasting is necessary. So it should not be used in this way.

@ilevkivskyi
Copy link
Member

I think this is a consequence of how list.__add__ is typed in typeshed. I am however not sure what would be a better signature.

@JelleZijlstra
Copy link
Member

python/typeshed#2404 fixes this.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 12, 2018

(This comment is copied from #5778, which is a duplicate issue.)

Code like this doesn't type check correctly (Python 2 example):

from typing import List, Optional
result = range(10) + [None]  # type: List[Optional[int]]

It generates these errors:

t.py:2: error: Incompatible types in assignment (expression has type "List[int]", variable has type "List[Optional[int]]")
t.py:2: error: List item 0 has incompatible type "None"; expected "int"

Issues like these seem somewhat frequent. There are also similar examples in the mypy codebase.

The errors have two causes:

  1. list concatenation cannot vary the return type based on context
  2. the types of the list items must match in concatenation.

A quick way to fix this particular example would be to change the signature of list.__add__ to something like this:

class list(Generic[_T]):
    ...
    def __add__(self, x: List[_S]) -> List[Union[_T, _S]]: ...
    ...

This signature might cause other problems, though. This wouldn't solve the problem in general. For example, if the annotation would say # type: List[Union[int, str, None]] the error would persist, even though the code would be safe.

With some type system extensions we could fix this more generally. We'd need type variable lower bounds (we currently only have upper bounds), and to allow type variable bounds to refer to other type variables. I think that this would be a good signature:

T = TypeVar('T')
S = TypeVar('S')
U = TypeVar('U', lower_bound=Union[T, S])

class list(Generic[T]):
    def __add__(self, other: List[S]) -> List[U]: ...

Implementing this could be difficult.

Instead, we could special case list.__add__ in the type checker, since it's a very common operation. This would be ugly and potentially confusing, since the effective signature wouldn't match typeshed.

@JukkaL JukkaL changed the title Concatenating lists of different derived classes' objects does not result in type upcast List concatenation problems with type context and different operand item types Oct 12, 2018
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 12, 2018

Increasing priority to high since we have several independent sightings of this issue.

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 29, 2018

python/typeshed#2404 caused regressions (see #5971 and #5874). I'm going to propose reverting it, since there seems to be no easy way to avoid the regressions in mypy. In summary, the PR fixed some issues but introduced others, which looks like a significant regression for existing mypy users.

If we want to fix the original issue, the simplest way would likely be to special case list.__add__ using a plugin, while preserving the old signature in typeshed. Not sure whether this is a big enough issue for a plugin to be worth it.

JukkaL added a commit to JukkaL/typeshed that referenced this issue Nov 29, 2018
The fix caused regressions for mypy that are difficult to
fix.  See python/mypy#5492 for
context.

This reverts commit 1a42a2c.
JelleZijlstra pushed a commit to python/typeshed that referenced this issue Nov 29, 2018
The fix caused regressions for mypy that are difficult to
fix.  See python/mypy#5492 for
context.

This reverts commit 1a42a2c.
yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this issue Jan 23, 2019
yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this issue Jan 23, 2019
The fix caused regressions for mypy that are difficult to
fix.  See python/mypy#5492 for
context.

This reverts commit 1a42a2c.
@WillAyd
Copy link

WillAyd commented Oct 24, 2019

Just wanted to comment here as we run into this issue a lot in pandas. Here's a specific example:

pandas-dev/pandas#29205 (comment)

There doesn't appear to be a good way to say add lists and pass as an argument to a function all in one go. Instead we either have to break this apart into a few lines and use append/extend OR use unpacking like [list1, *list2] to make things work. The former gets very verbose the more lists you have, and the latter seems a less natural way to add lists together

Understood limitations that caused this to be reverted in the first place; wanted to voice support for this issue in case someone has a good idea on how to make it work going forward!

@lhupfeldt
Copy link

lhupfeldt commented Feb 26, 2021

I was going to open an issue after getting an issue with list concatenation, but but I think it is a variation of this.

from typing import Iterable, Union

aa: Iterable[str] = ["a"]
bb: Iterable[Union[str, int]] = [1]


def strs_and_ints(xx: Iterable[Union[str, int]]):
    print(list(xx))

strs_and_ints(aa)
strs_and_ints(list(aa))
strs_and_ints(list(bb) + list(aa))

# False positives. The argument type of strs_and_ints should be used to determine validity of list concatenation.
strs_and_ints(list(aa) + list(bb))
strs_and_ints(["a"] + list(bb))
mypy list_arg.py
list_arg.py:11: error: Argument 1 to "list" has incompatible type "Iterable[Union[str, int]]"; expected "Iterable[str]"
Found 1 error in 1 file (checked 1 source file)
``

@twoertwein
Copy link

seems like mypy 0.981 might have (accidentally?) fix this issue.

@AlexWaygood
Copy link
Member

seems like mypy 0.981 might have (accidentally?) fix this issue.

Well, not exactly an accident, given how long we spent discussing it in python/typeshed#8293 ;)

But yup, this is fixed now! 🎉

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

Successfully merging a pull request may close this issue.

8 participants