-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Mixins with generic self and type bounds #7191
Comments
I am having the same issue with the mixins I wrote for Django models, e.g.: T = TypeVar("T", bound=models.Model)
class SafeCreateMixin:
@classmethod
@transaction.atomic()
def safe_create(cls: Type[T], **kwargs) -> Optional[T]:
try:
return cls.objects.create(**kwargs)
except IntegrityError as e:
if not is_unique_violation(e):
raise
return None It fails with the following error:
|
These kind of restrictions on |
See e.g. python/typeshed#3110 (comment) for other uses of explicit |
@zero323: |
@ziima Fair point, but I am not convinced that such transitive notion has any practical applications. It would effectively mean that the type check on Personally I'd say that a specific error, that can be selectively suppressed, is much more useful in such case, i.e.: class Baz(MixIn): ... # Error: Cannot prove that "Baz" is a subclass of "Foo" [self-bound] I'd say that if you really want to type check such cases self bound should be explicit, i.e. class Foz(MixIn):
def foz(self: T) -> None: ... but that might unnecessarily complicate the implementation. |
It does. Inheritance of mixins is generally used, for example it's heavily used in Django views https://github.com/django/django/blob/master/django/views/generic/edit.py I'd prefer a way, where I can declare class as a mixin and specify the class it should be used with. |
I am pretty sure we are not talking about the same thing here. I never suggested that either compost ion or hierarchies of mix-ins are not used in practice. What I am saying is that supporting such cases using transitive semantics is not practical in context of lightweight self annotations (and probably any other case, without explicit hints for all extending classes). In particular in many cases it would mean that such annotation is as good as |
No, not even there. We should only give error when Baz().foo() # Invalid self type "Baz" for "foo", expected "Foo" This is just Python works, it actually passes |
Another important question here is what to do if one wants to use an attribute of the mixin class in the mixin method body. One possible solution is to implicitly assume the type of This makes me think maybe we should introduce class-only intersections? I don't think one would ever need to intersect a callable and a tuple. Essentially, this class-only intersection would obey the same rules as I am not sure however about type variables, @JukkaL what do you think? |
Fixes python#3625 Fixes python#5305 Fixes python#5320 Fixes python#5868 Fixes python#7191 Fixes python#7778 Fixes python/typing#680 So, lately I was noticing many issues that would be fixed by (partially) moving the check for self-type from definition site to call site. This morning I found that we actually have such function `check_self_arg()` that is applied at call site, but it is almost not used. After more reading of the code I found that all the patterns for self-types that I wanted to support should either already work, or work with minimal modifications. Finally, I discovered that the root cause of many of the problems is the fact that `bind_self()` uses wrong direction for type inference! All these years it expected actual argument type to be _supertype_ of the formal one. After fixing this bug, it turned out it was easy to support following patterns for explicit self-types: * Structured match on generic self-types * Restricted methods in generic classes (methods that one is allowed to call only for some values or type arguments) * Methods overloaded on self-type * (Important case of the above) overloaded `__init__` for generic classes * Mixin classes (using protocols) * Private class-level decorators (a bit hacky) * Precise types for alternative constructors (mostly already worked) This PR cuts few corners, but it is ready for review (I left some TODOs). Note I also add some docs, I am not sure this is really needed, but probably good to have.
Wow, what a great PR. 👏 |
@zero323 what solution did you end up with? One solution could be writing |
@mr-bjerre I am afraid that, after some half-hearted attempts of implementing a plug-in, I decided I could live without it. Still, I think it would be a nice thing to have in general. |
This issue is closed but the last example still raises (with 910)
Is there a solution to this or is this a wontfix? For completeness: from typing import TypeVar
class Foo: ...
T = TypeVar("T", bound=Foo)
class MixIn:
def foo(self: T) -> T: ... |
You can make it a protocol: from typing import TypeVar, Protocol
class Foo(Protocol):
host_attr: int
T = TypeVar("T", bound=Foo)
class MixIn:
def foo(self: T) -> T:
self.host_attr += 1 # OK
return self
class Host(MixIn):
host_attr: int
class BadHost(MixIn):
host_attr: str
Host().foo() # OK
BadHost().foo() # Error: Invalid self argument "BadHost" to attribute function "foo" with type "Callable[[T], T]" |
Thanks! |
Let's say I have a class
and a typevar with bound
Now I'd like to define a class that will be used as a mixin:
I want to express two things here:
MixIn
should be used only as a mixin withFoo
(the idea is similar to Scala self-type. In other words this should type check:while this shouldn't:
MixIn.foo
returns the same type asself
(at leastFoo
withMixIn
).Additionally it should address issues like #5868 or #5837 with zero boilerplate.
Right now (0.720+dev.e479b6d55f927ed7b71d94e8632dc3921b983362) this fails:
The text was updated successfully, but these errors were encountered: