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

Reason given for disallowing non-concrete subtype assignment is unsound #1647

Open
ippeiukai opened this issue Mar 5, 2024 · 10 comments
Open
Labels
topic: documentation Documentation-related issues and PRs

Comments

@ippeiukai
Copy link

ippeiukai commented Mar 5, 2024

Issue

According to the discussion #1305, “type checkers allow incompatible __init__ overrides, because flagging them would be too disruptive.”

Accepting above as de facto, the example given as the main reason for disallowing non-concrete subtype assignment in the following section of the spec is unsound:
https://github.com/python/typing/blob/e08290b70f58df509f998cbbe09a8e65abb57a9b/docs/spec/protocol.rst#type-and-class-objects-vs-protocols

class Proto(Protocol):
    @abstractmethod
    def meth(self) -> int:
        ...
class Concrete:
    def meth(self) -> int:
        return 42

def fun(cls: type[Proto]) -> int:
    return cls().meth() # ???
fun(Proto)              # Why should this error?
fun(Concrete)           # OK

var: Type[Proto]
var = Proto    # Why should this error?
var = Concrete # OK
var().meth()   # ???

(credit python/mypy#4717 (comment) for pointing out the contradiction)

Thoughts

One radical approach would be to remove the concreteness rule from the spec altogether. The type of Proto is type[Proto], and if the constructor compatibility is not checked, there is little reason to disallow it from being assigned to variables annotated as type[Proto].

If that is too radical, the spec can stay as is but the reasoning and associated examples should be clearly marked ‘historical’ and no longer valid. That way, we can avoid any immediate changes in practice, but at the same time encourage discussions towards appropriate future specs.

@ippeiukai ippeiukai added the topic: documentation Documentation-related issues and PRs label Mar 5, 2024
@ippeiukai ippeiukai changed the title Signatures of __init__ method overrides are exempted from checks in major type checkers Reason given for disallowing non-concrete subtype assignment is unsound Mar 5, 2024
@NeilGirdhar
Copy link

Thanks for posting this. I suggest posting this to the discuss, which should attract more attention.

@ippeiukai
Copy link
Author

@NeilGirdhar
Copy link

FYI discuss is here: https://discuss.python.org/

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 6, 2024

I think type[ProtocolType] often doesn't make sense. In the example above, you're better off with Callable[..., ProtocolType] — this clearly shows you that there can be no one way to instantiate a structural type.

Also see cases like python/mypy#16919 / python/mypy#16890

I'm curious what the real world code where you're encountering issues looks like?

@ippeiukai
Copy link
Author

I'm curious what the real world code where you're encountering issues looks like?

Please check this issue: python/mypy#4717

Any function that accepts a type object as its argument and returns an instance of that type gets affected by the concreteness restriction. In my experience, DI containers really struggle with this because they are supposed to facilitate binding of implementation to abstraction:

@hauntsaninja
Copy link
Collaborator

Thanks for the links, both of those look like cases of (type[T]) -> T where T is a TypeVar, not a Protocol. Sounds like you want something more like python/mypy#9773

@ippeiukai
Copy link
Author

both of those look like cases of (type[T]) -> T where T is a TypeVar, not a Protocol.

I think T can bind to Proto to make (type[Proto]) -> Proto without a problem. The issue is that such function does not accept Proto to be specified as its parameter because Proto is a non-concrete type even though type of Proto is type[Proto].

https://github.com/python/typing/blob/e08290b70f58df509f998cbbe09a8e65abb57a9b/docs/spec/protocol.rst#type-and-class-objects-vs-protocols

Variables and parameters annotated with type[Proto] accept only concrete (non-protocol) subtypes of Proto.

@ippeiukai
Copy link
Author

ippeiukai commented Mar 7, 2024

python/mypy#9773 can somewhat improve the situation, but not sure DI containers would want to accept any TypeForms than just concrete and non-concrete Types.

Probably the AbstractType idea here works better.

But, anyway, when the main reason for having concreteness restriction is unsound, why do we keep it at all?

@NeilGirdhar
Copy link

@ippeiukai I believe Eric brought up your issue here in case you wanted to participate!

@ippeiukai
Copy link
Author

@NeilGirdhar Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Documentation-related issues and PRs
Projects
None yet
Development

No branches or pull requests

3 participants