-
-
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
Metaclass type/instance difficulties #3625
Comments
Related to #3346 and some other discussions which I can't seem to find right now.
I think your first error message does not show up on master. However, it does show up if you constrain your variable "properly": T = TypeVar('T', bound='Model') The reason is that it is unsafe: class ModelMeta(type):
def build(cls: Type[T]) -> T: return cls()
class Model(metaclass=ModelMeta): ...
class NonModel(metaclass=ModelMeta): ...
NonModel.build() # error - this is not `Type[T]` But I think @ilevkivskyi argued that this slight unsafety is useful, as in your case. If the metaclass was somehow denied of being used outside the file, it could be even possible to check at the point of definition, and not just access. Intersection types can only help in the implementation of the function, but at some point the arguments should be checked to see that they are of Maybe a constraint on TypeVar could have helped: T = TypeVar('T', metaclass='ModelMeta') Or perhaps something like class ModelMeta(type):
def build(cls: ModelMeta[T]) -> T: ... But I'm not sure exactly what should it mean. Maybe that's equivalent to your first wish. I'm just thinking out loud. |
Concerning this example: T = TypeVar('T', bound='Model')
class ModelMeta(type):
def build(cls: Type[T]) -> T: return cls()
class Model(metaclass=ModelMeta): ...
class NonModel(metaclass=ModelMeta): ...
NonModel.build() # error I think this can be made safe if mypy will give the error at the point of use, not at the point of definition of Concerning the plugin hook, we support many plugin hooks, but I am not sure about his one. @JukkaL are there plans to support plugins for |
I'm not against the idea of checking on access, but it comes with some cost:
|
Mypy could also reject the class There are currently no concrete plans to support subtyping plugin hooks, but the plugin system is still experimental and we can add all sorts of new hooks if they seem useful. @rtpg Feel free to create issues for any new plugin hooks that you would find useful, preferably with examples of how you'd use them. |
Interesting... and it solves points (1) and (2) from my earlier comment. |
I hadn't thought too deeply about adding bounds to the Though in my case, knowing that I will have at least a partial solution in #3346 helps me to solve most of my worries about the type here. |
@elazarg I just pulled in
The error messages I get here are:
The first one is the system "working as intended". But I'm not quite sure what to do about those method type error messages |
with #3227 you get the following error: |
ah sorry, I read your comment as saying that if I don't add bounds to the type var, then this could be setup. This is a bit frustrating, because I feel like I'm so close to getting type checking on a pretty big part of the Django ORM but am hitting this issue, even though it feels like mypy "gets it". I'm trying to figure out an alternative way to tackle this issue by having the type variable be on the
In this I also get the method failures, I imagine the same erased type issue is happening in this case as well? |
Another similar issue: import typing
T = typing.TypeVar('T', bound='Foo')
class FooMeta(type):
def __repr__(self: typing.Type[T]) -> str:
return self.name
class Foo(metaclass=FooMeta):
name = 'FooClass'
|
Technically there are two issues in your example: the first is the discussed issue (there is no guarantee that The second issue is that there is no reason to assume that |
Ehm,
|
Indeed. Perhaps I'm confusing |
Note that this is closely related to #7191 (in the sense that the solution is the same, move consistency check from definition to use site, i.e. to |
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.
Consider the following code from my hypothetical ORM that has three different ways of building an object
I'm having trouble defining
build
in a way that type checks. In this example, I get an error onbuild
about howType[T]
is not a supertype of the classModelMeta
.build_other
passes the type checker, but usages don't seem to catch anything (implicit Any perhaps?)Considering class methods work, it feels like metaclass methods should equally be able to "unwrap" the instance type for a class, but there might be subtleties I'm missing here.
My wish list here would be:
Type
(something likeInstance
) so I could write something likedef build(cls: T) -> Instance[T]
. This would be (I think) a bit clearer.def build(cls: (Type[T] & ModelMeta)) -> T
. This would help with a lot of other problems as well. This seems like it would be a useful shortcut to writing subclassessubtypes.is_subtype
that would let me write a custom subtype check in some typesFull context: I'm writing some stubs for Django's ORM, and was hitting issues around the type of
Model.objects.create
. So in my case I end up needing a "class property" (hence doing things through the metaclass), which is why the simplenew()
call strategy hasn't been working for me.I think this is related to #3438, but I'm having a hard time identifying the link.
The text was updated successfully, but these errors were encountered: