Update Apps.get_model() return type

[false-vacuum]

I think this solves the problem for everyone.

Related issues

• Fixes: Too broad return value for apps.get_model() #304
• Closes: Fix apps.get_model() return type #305
• Closes: Make Apps.get_model() return Type[Model] #814

[intgr]

Hi. I don't think it can work like this. The problem is that the model class name is passed in as string to model_name argument.

With this construction I suspect it will end up being type[Any] as well.

[false-vacuum]

This works with the static type checker built into pycharm.

[false-vacuum]

After making this change

---

Before

[intgr]

If all you want is the Model type, not the correct subclass of Model, then we could use type[Model] as return, no need for this complexity with TypeVar.

But you usually need the correct Model subclass, for example User, with its managers an fields etc. Right now you can use model_class: type[User] = apps.get_model(...), but if you make this return type[Model] then you would always need an explicit cast() or assert isinstance().

It used to be type[Model], but was explicitly reverted with a comment in #109:

d53121b#diff-90641780bd3e00fc85922341c39871903999c6e562992cfc2b065550de1debafL26-R28

I'm personally undecided whether type[Any] or type[Model] is better. Maybe @mkurnikov as author of the PR can chime in on why the former was preferred.

[mkurnikov]

This specific annotation could be automatically generated, or I just put something in as I didn't focus much of this specific method.

Proper implementation should have custom support from the codebase, it's not typeable just from annotations. See get_user_model() support for example, it has similar properties.

[mkurnikov]

For the PyCharm: interop with PyCharm is extremely hard without close collaboration, they have all kind of special-casing for the models inside the plugin. So do we. If we want to support it properly, we need to design it together, let's maybe ask
@vlasovskikh ?

[false-vacuum]

If all you want is the Model type, not the correct subclass of Model, then we could use type[Model] as return, no need for this complexity with TypeVar.

But you usually need the correct Model subclass, for example User, with its managers an fields etc. Right now you can use model_class: type[User] = apps.get_model(...), but if you make this return type[Model] then you would always need an explicit cast() or assert isinstance().

It used to be type[Model], but was explicitly reverted with a comment in #109:

d53121b#diff-90641780bd3e00fc85922341c39871903999c6e562992cfc2b065550de1debafL26-R28

I'm personally undecided whether type[Any] or type[Model] is better. Maybe @mkurnikov as author of the PR can chime in on why the former was preferred.

If I'm understanding pep484 correctly, the problem with type[Model] in this case is that by default type declarations are considered invariant.
So if you wanted to do something like this:

def some_function(model: SomeModel):
   pass

cls = apps.get_model('aws_manager', 'SomeModel')

some_function(cls)   # type check error here

The type checker would report: "Expected type 'SomeModel', got 'Type[Model | Model]' instead" when calling some_function(cls).

Using TypeVar with the bound parameter tells the type checker that the return type can be a subclass of Model and not just Model. I'm not sure if this is the only way to do this, but it does seem correct.

That being said, type[Model] would definitely be preferred over type[Any], because it at least it would allow you to access the manager without reporting an error.

[intgr]

I did some tests with this on mypy and PyCharm.

type[TypeVar(...)] is an invalid construction. Mypy doesn't report this as error, but PyCharm does:

You can tell that mypy doesn't handle it correctly because it doesn't report errors here, it's being treated as Any:

from django.apps.registry import Apps

def takes_int(value: int) -> None:
    pass

def bla(apps: Apps) -> None:
    model_type = apps.get_model("auth", "User")
    reveal_type(model_type)  # Mypy: note: Revealed type is "ModelType?"
    takes_int(model_type)  # <-- This should be error, but no error is reported
    model_type.no_such_attribute = 123  # <-- No error reported

---

If I change it like this:

-ModelType = type[TypeVar("ModelType", bound=Model)]
+ModelType = TypeVar("ModelType", bound=type[Model])

Then mypy actually does something slightly better, it asks for an explicit type hint for model_type variable:

def bla(apps: Apps) -> None:
    model_type = apps.get_model("auth", "User")  # Mypy: error: Need type annotation for "model_type"
    reveal_type(model_type)  # Mypy: note: Revealed type is "Any"
    takes_int(model_type)  # <-- Still no error reported

But I can hint it as model_type: int and it will just accept it without any errors 🤷

def bla(apps: Apps) -> None:
    model_type: int = apps.get_model("auth", "User")  # <-- No error
    reveal_type(model_type)  # Mypy: note: Revealed type is "builtins.int"
    takes_int(model_type)  # <-- Also no error

Arguably it should infer the type as type[Model] from TypeVar's bound and report it as incompatbile with int. But that's not what happens right now. For TypeVars to be useful, there needs to be a function parameter using it also: https://mypy.readthedocs.io/en/stable/generics.html#generic-functions

For now, all of these seem to boil down to being equivalent to type[Any].

---

Is there anything I'm missing?

[intgr]

As for why this change causes PyCharm to offer better autocompletion suggestions, I don't know, but it doesn't seem to affect the type checking.