-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix collections.abc.Callable
and typing.Callable
#14761
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
2ae6722
to
11077b4
Compare
89ced89
to
17f5d58
Compare
This comment has been minimized.
This comment has been minimized.
I'm always in favor of bringing our stubs more in line with reality, so in general I'm in favor of the change. But unfortunately at least mypy seems heavily dependent on the current typing – as shown by the mypy primer output. We'd need to ensure that current type checkers will work with the updated stubs, before making this change. Maybe it's also worth trying both changes (to |
this is a circular dependency. how can we ever change anything if the change needs to be compatible with our dependants i will try them in isolation, i feel that the class with generics isn't going to be so well consumed, although i would be interested in pulling in the maintainers of other type checkers to see if they are welcomming to the change (fyi, i maintain pycharm) |
17f5d58
to
0abae9e
Compare
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
In that case this needs coordination with the dependents, but we won't change this unilaterally. Just changing |
That's why I said in astral-sh/ty#1215 (comment):
It's not impossible to make a change like this but, as @srittau says, it is nontrivial. You'll first need to get consensus among all the major type checkers that the change is worth making (or at the very least, make sure that they're aware the change is coming, and make sure that they're willing to adapt to the change). Ideally we'd have PRs "ready to go" in the type checkers that need changes before the typeshed change is merged. |
so, any advice on resolving these failing tests? should we summon the maintainer of pyright? (or basedpyright?) |
Yes, @erictraut might be able to help. |
# Class for defining generic aliases for library types. | ||
def __getitem__(self, typeargs: Any) -> Any: ... | ||
|
||
Callable = _Alias() |
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.
this does sort of make me wonder... an alias to what? For all other _Alias
es in typeshed, ty is able to defer to an "actual class somewhere else" for that _Alias
's semantics in type annotations etc.. But here there is no "actual class somewhere else" -- collections.abc.Callable
is just a re-export of this symbol.
(I realise that you've done it this way because you tried the first way and it was much more disruptive...)
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, i have addressed this in a comment in the "Conversion" tab
one option would be to introduce a _Callable[**P, R]
definition
This is a pretty disruptive change. Is there a specific problem that it solves? I'm not against making improvements if there's a real benefit to users or type checker authors, but it's not clear to me what those benefits are in this case. As @AlexWaygood mentioned, if we want to model But then how is |
primarily that from collections.abc import Callable
def f(t: type[object]): ...
f(Callable)
class C(Callable): ...
a: Callable
a.__call__
what exactly are the different rules to a normal class? i'm not aware of any, i would expect: class Callable[**Parameters, Return](Protocol): # not actually a protocol, but acts like one, same as everything else in collections.abc
@abstractmethod
def __call__(self, *args: Parameters.args, **kwargs: Parameters.kwargs) -> Return: ... how much effort would be involved in un-special casing so i see two options:
|
I'm still trying to understand if the proposed change is solving an actual problem that users are hitting or if this falls more under the category of "it would be nice if this were more aligned with the runtime"? If it's the latter, then I think we need to consider the impact and cost of the change and weigh it against the benefits.
I can't speak for other type checkers, but for pyright it's not as simple as "un-special casing". |
i have run into these issues when trying to write code additionally, it is very confusing to see that the definition of
i don't think there is anything special about these semantics at all: Code sample in basedpyright playground from collections.abc import Callable
from typing import Concatenate
class CustomCa[**P, R]: ...
_ = Callable[..., int]
_ = CustomCa[..., int]
def f[**P](
a: Callable[[Concatenate[int, P]], None],
b: CustomCa[[Concatenate[int, P]], None],
): ... the only special casing that i am aware of is the assumption that |
typing.Callable
is not a_SpecialForm
, it's an_Alias
, andcollections.abc.Callable
is a classi is trying to address many issues with
Callable
that are either 100% special cased, or don't currently workstatus
undecided on how to proceed, two options present:
typing.Callable
and introduce_collections_abc._Callable[**P, R]
. this is less disruptive (i think)typing.Callable
and introduce_collections_abc.Callable[**P, R]
, this is potentially more disruptive, but more correct