-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix list.copy
to use the expected type
#14755
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
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
7a3f21d
to
b81d98d
Compare
This comment has been minimized.
This comment has been minimized.
@overload | ||
def __init__(self, iterable: Iterable[_T], /) -> None: ... | ||
def copy(self) -> list[_T]: ... | ||
def copy(self) -> list[_S | _T]: ... |
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.
_S
has no meaning, as it's only used once in this context. It's just interpreted as Any
in this context.
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 see what you're trying to do, but we should probably use:
def copy(self) -> list[_S | _T]: ... | |
# The new list can allow broader types that the old one. `Any` allows that. | |
def copy(self) -> list[_T | Any]: ... |
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.
_S
has no meaning, as it's only used once in this context. It's just interpreted asAny
in this context.
that is demonstrably not true
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.
Code sample in basedpyright playground
from __future__ import annotations
from typing import Any
class A[T]:
t: T
def copy1[Expected](self) -> A[Expected | T]:
return A()
def copy2[Expected](self) -> A[Any | T]:
return A()
a: A[object] = A[int]().copy1()
a: A[object] = A[int]().copy2()
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.
basedpyright is incorrect there. A[int | Any]
should be assignable to A[object]
.
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.
consider this example:
def empty_list[T=Never]() -> list[T]:
return []
a = empty_list()
b: list[int] = empty_list()
if we were to use Any
here, then the type of a
would be list[Any]
additionally, there would be absolutely no safety within empty_list
to return the correct type. i'm not really sure where this idea that type variables only in return positions being invalid came from
this is literally the meaning of a type variable, to use Any
is a mistake
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.
basedpyright is incorrect there.
A[int | Any]
should be assignable toA[object]
.
yes: microsoft/pyright#10713 (comment)
i was a little hasty with the first example, i think it's clear though that this case is the intended use of a type variable, not Any
b81d98d
to
4ac3aa3
Compare
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
support: