Restore ability to instantiate, access objects manager, of abstract models

[intgr]

Continuing the discussion started at #1649 (comment)

There are two similar use cases affected by two different PRs:

• Fix crash when constructing an abstract model with recursive relation #1393
• Move ModelBase.objects declaration to Model.objects, for mypy 1.5.0 #1649

Since the changes in #1393, django-stubs will raise errors on some kinds of generic code that handles abstract models. For example:

# models.py
from typing import TypeVar
from django.db import models
from django_stubs_ext.db.models import TypedModelMeta

class Animal(models.Model):
    name = models.CharField(max_length=100)

    class Meta(TypedModelMeta):
        abstract = True

class Cat(Animal):  # Concrete model
    pass

def create_animal(klass: type[Animal], name: str) -> Animal:
    obj = klass(name=name)
    obj.save()
    return obj

T = TypeVar("T", bound=Animal)

def create_animal_generic(klass: type[T], name: str) -> T:
    return klass.objects.create(name=name)

# Usage example:
# create_animal(Cat, "Garfield")
# create_animal_generic(Cat, "Grumpy")

These two functions are allowed and work with django-stubs 4.2.3. Function create_animal causes errors in git master version of django-stubs:

apidoc/models.py:16:11: error: Cannot instantiate abstract model "Animal"  [misc]

But after the unmerged changes in PR #1649, the second function create_animal_generic will also start failing:

models.py:16:11: error: Cannot instantiate abstract model "Animal"  [misc]
models.py:23:12: error: Cannot instantiate abstract model "Animal"  [misc]
models.py:23:12: error: Incompatible return value type (got "Animal", expected "T")  [return-value]

System information

• python version: 3.11.4
• mypy version: 1.4.1
• django-stubs version: 4.2.3 / git

[intgr]

A possible work-around is using "TypeVar value restrictions" instead of bound=. But it's somewhat annoying and requires enumerating all possible subclasses of Animal and thus won't work for libraries that are supposed to be extended by the user.

Example:

class Cat(Animal):
    pass

class Dog(Animal):
    pass

T = TypeVar("T", Cat, Dog)  # <-- must list all possible Animal subclasses

def create_animal_generic(klass: type[T], name: str) -> T:
    return klass.objects.create(name=name)

This code checks succesfully even after changes in #1649.

[sobolevn]

I vote -1 on any TypeVars with values :)

[intgr]

Pinging @flaeppe @andersk as related PR authors.

[intgr]

I think I have a working fix for the create_animal_generic variant in https://github.com/typeddjango/django-stubs/pull/1649/files#r1291187079

I will submit a PR with a testcase as well, so we don't regress it in the future.

[intgr]

Welp, the proposed fix would cause other issues to appear (when overriding objects with a custom manager). I'm at a loss here.

[andersk]

mypy is correct to complain about your create_animal and create_animal_generic functions, because their signatures allow calls with an abstract model that will fail at runtime:

create_animal(Animal, "Twilight Sparkle")
create_animal_generic(Animal, "Princess Celestia")

How about rewriting them to accept a manager rather than a type object?

T = TypeVar("T", bound=Animal)

def create_animal_by_manager(objects: models.Manager[T], name: str) -> T:
    return objects.create(name=name)

create_animal_by_manager(Cat.objects, "Grumpy")

[flaeppe]

I think andersk conclusion is correct.

I don't think it's possible to restrain a TypeVar to only accept subclasses of a given bound? Except for explicitly typing out each subclass in TypeVar values

A slightly different attempt could be to declare a protocol, describing an expected objects attribute. Essentially the same as andersk suggested but still possible to pass a model type

Though I haven't had the best experience using models with protocols..

[flaeppe]

Hm, would it work to declare a protocol that describes the inheritance and use that as bound? I.e.

class XYZ(Animal, Protocol): 
    ...

T = TypeVar("T", bound=XYZ)

Or am I completely off here?

Edit: Might not even be needed with a TypeVar

[flaeppe]

I've tried to dig around a bit into what mypy does for abstract base classes (abc.ABC) and I from what I can tell they cover an additional case, for error named type-abstract(this permalink) looking like:

Only concrete class can be given where "type[...]" is expected  [type-abstract]

See the following playground link for how to trigger the error: https://mypy-play.net/?mypy=latest&python=3.11&gist=e68d5eeca6994812788f81ae69a3967a

Essentially there's an additional case which yields errors on corresponding calls mentioned by andersk, if we assume Animal to be an abstract base class(abc.ABC)

create_animal(Animal, "Twilight Sparkle")  # E: Only concrete class can be given where "type[Animal]" is expected  [type-abstract]
create_animal_generic(Animal, "Princess Celestia")  # E: Only concrete class can be given where "type[Animal]" is expected  [type-abstract]

Now, Django also guards abstract models by raising a: TypeError: Abstract models cannot be instantiated. Much like Python's abc.ABC.

So, perhaps we could attempt to mimic what mypy does with an abc.ABC type and yield an error passing an Animal abstract type as a function argument?

[flaeppe]

Forgot to mention that if the guard I mention is introduced, we could safely try to exclude yielding an error for type[Animal] inside the function body on klass.objects.create(name=name). Since it's then no longer allowed to call any of the functions with Animal as an argument.

[sobolevn]

Yes, I've also experimented with mimicin abstract models to be abstract classes, but something didn't work for me :)

Oh, I remember: is_abstract in mypy means that this class would have abstract_attributes set. But, abstract models do not have any specific attributes to set.

Please, feel free to experiment - I would love to see this feature.

[flaeppe]

I've opened #1663, including some kind of attempt to mimic abstract base classes

[flaeppe]

Yes, I've also experimented with mimicin abstract models to be abstract classes, but something didn't work for me :)

Oh, I remember: is_abstract in mypy means that this class would have abstract_attributes set. But, abstract models do not have any specific attributes to set.

Please, feel free to experiment - I would love to see this feature.

I think I got it to work by adding 2 "abstract attributes" I found, from Django's model meta class: <MyModel>.DoesNotExist and <MyModel>.MultipleObjectsReturned. Not truly abstract attributes perhaps but at least they only exist on concrete models.

We should probably only add those 2 attributes to concrete models, with the plugin..