django-stubs icon indicating copy to clipboard operation
django-stubs copied to clipboard

AbstractUser lacks Meta-attribute

Open prauscher opened this issue 1 year ago β€’ 9 comments

Bug report

What's wrong

Since version 5.0.0 (probably since #2000), the following code fails:

from django.contrib.auth.models import AbstractUser

class User(AbstractUser):
    class Meta(AbstractUser.Meta):  # error: Name "AbstractUser.Meta" is not defined
        pass

obviously this example is shortened, but I think you get the Idea. While it is true that a basic Model does not have a Meta-attribute, this is not valid for basically any Subclass for Meta, and using this inheritance is actually the recommended way by django docs:

  • https://docs.djangoproject.com/en/dev/topics/db/models/#meta-inheritance
  • https://docs.djangoproject.com/en/dev/topics/auth/customizing/#using-a-custom-user-model-when-starting-a-project

How is that should be

AbstractUser.Meta should exist for inheritance.

System information

  • OS: Linux (Alpine and Arch tested, but the problem should be valid regardless)
  • python version: 3.12
  • django version: 5.0.4
  • mypy version: 1.10.0
  • django-stubs version: 5.0.0
  • django-stubs-ext version: none

prauscher avatar May 02 '24 07:05 prauscher

@jorenham This is caused by #2000, any thoughts? I think we should do something here.

I don't understand why for example AbstractUser.Meta is accessible, but Permission.Meta is not.

>>> from django.contrib.auth.models import Permission
>>> Permission.Meta
Traceback (most recent call last):
  File "<console>", line 1, in <module>
AttributeError: type object 'Permission' has no attribute 'Meta'. Did you mean: '_meta'?
>>> from django.contrib.auth.models import AbstractUser
>>> AbstractUser.Meta
<class 'django.contrib.auth.models.AbstractUser.Meta'>

Even though model class Permission does have class Meta: inner class (?) Is this something about abstract = True models?

intgr avatar May 04 '24 14:05 intgr

So there are two ways to define the metaclass of a model that inherits from another model (it doesn't matter if it's abstract or not).

So given

class Dad(models.Model):
    class Meta: ...

Option 1

class Child(Dad):
    class Meta: ...

This is what django-stubs implicitly assumes since 5.0.

Advantages:

  • This will always work, even if the parent model doesn't define a Meta class.
  • Avoids having to deal with Django's magical abstract, which is both True and False (quantum mechanics is difficult)

Disadvantages:

  • But this is not very DRY in case you want to e.g. use the same verbose_name as the base model.
  • Typecheckers don't like it if both parent and child model define a (properly annotated) inner Meta. See this pyright playground example.

Option 2

class Child(Dad):
    class Meta(Dad.Meta): ...

Advantages:

Disadvantages:

  • It will require all child models with a Meta to inherit the meta from it's base class, but only if the parent model actually has a Meta. If done incorrectly, it will break your entire project.
  • It's not obvious how to deal with ~SchrΓΆdinger's cat~ abstract in the stubs (include it as a bool, Literal[False], Literal[True], or just ignore it).

From a typing perspective, option 2 seems to be the least incorrect.

But perhaps option 1 might be easier: I suspect (based on a vague recollection of 3rd party code of django plugins I've seen over the years) that most django users aren't using Meta inheritance very often.

Personally, I'd prefer option 2. But keep in mind that I'm the kind of person that can watch a football match without saying a word, but starts cursing out loud after seeing that AbstractUser.Meta is missing from the stubs 🀷🏻

jorenham avatar May 04 '24 23:05 jorenham

Not too sure iff AbstractBaseUser does have anything to do with it, but one difference between Permission and AbstractUser is that Permission is a direct child of models.Model, while AbstractUser inherits from AbstractBaseUser first.

Anyway, in my opinion option 2 must be supported, as it is the one suggested by django docs - although it sure is not used too often, when used according to docs there should not be faults given by the typechecker for perfectly working code.

prauscher avatar May 05 '24 22:05 prauscher

@prauscher AbstractBaseUser wasn't mentioned?

jorenham avatar May 05 '24 23:05 jorenham

Sorry, I only read now how my comment could be miss-interpreted: I was just speculating iff AbstractBaseUser could have anything to do with it, since one difference between Permission (lacking Meta) and AbstractUser (having Meta) is that AbstractUser is not a direct child of models.Model, but inherits from AbstractBaseUser. From the code I do not see any hints why this could be, but just wanted to share this insight.

prauscher avatar May 07 '24 10:05 prauscher

@prauscher Ah I see now. But I believe that this issue in particular is caused by the fact that AbstractUser.Meta isn't present in the stubs, even though it exists at runtime.

jorenham avatar May 11 '24 23:05 jorenham

Anyway, let's just see which classes have .Meta available and which don't. And then just add to stub classes according to that. Don't have time right now to open a PR for that.

I created a hack script to enumerate all model classes and check if they have Meta. https://gist.github.com/intgr/352cac61b4f95e921bb33a0d4b76d324

Results...

βœ… django.contrib.auth.models.AbstractUser => AbstractUser.Meta ❌ No Meta: django.contrib.auth.models.Permission ❌ No Meta: django.contrib.flatpages.models.FlatPage ❌ No Meta: django.contrib.flatpages.models.FlatPage_sites ❌ No Meta: django.contrib.auth.models.Group_permissions ❌ No Meta: django.contrib.auth.models.Group βœ… django.contrib.auth.base_user.AbstractBaseUser => AbstractBaseUser.Meta ❌ No Meta: django.contrib.redirects.models.Redirect βœ… django.contrib.auth.models.PermissionsMixin => PermissionsMixin.Meta βœ… django.contrib.sessions.base_session.AbstractBaseSession => AbstractBaseSession.Meta ❌ No Meta: django.contrib.admin.models.LogEntry ❌ No Meta: django.contrib.auth.models.User_user_permissions βœ… django.contrib.sessions.models.Session => AbstractBaseSession.Meta ❌ No Meta: django.contrib.sites.models.Site βœ… django.contrib.auth.models.User => AbstractUser.Meta ❌ No Meta: django.contrib.contenttypes.models.ContentType ❌ No Meta: django.contrib.auth.models.User_groups

intgr avatar May 12 '24 10:05 intgr

I've also observed the same issue with forms, notably trying to use a custom User model with the auth forms.

from django.contrib.auth.forms import BaseUserCreationForm
from .models import User

class UserCreationForm(BaseUserCreationForm):
    class Meta(BaseUserCreationForm.Meta):  # error: Name "BaseUserCreationForm.Meta" is not defined  [name-defined]
        model = User
        fields = ["email"]

Anyway, let's just see which classes have .Meta available and which don't. And then just add to stub classes according to that.

Can we do the same for the Form classes? (I couldn't really follow why they were removed in #2000)

steele avatar Oct 16 '24 11:10 steele

@jorenham This is caused by #2000, any thoughts? I think we should do something here.

If SomeModel.Meta exists at runtime, then the stubs should reflect that. However, if it doesn't exist at runtime, then it also shouldn't exist in the stubs. The latter is what was fixed by #2000.

But it could very well be that we missed something in #2000, or that since then, django has changed this.

Either way, iff AbstractUser.Meta exist at runtime, then these should also exist in the stubs, and the same goes for BaseUserCreationForm.


I don't understand why for example AbstractUser.Meta is accessible, but Permission.Meta is not.

Yea to me it all feels rather inconsistent; it would've saved us a lot of trouble if all models would have had a .Meta by default


Even though model class Permission does have class Meta: inner class (?) Is this something about abstract = True models?

I think that the there are only two viable options for dealing with this abstract = True black voodoo magic:

  1. As discussed in #2000, if we were to annotate abstract: ClassVar[bool] or abstract: ClassVar = True in e.g. AbstractUser.Meta, then each subtype of AbstractUser.Meta will be required to explicitly (re-)define a abstract class attribute.
  2. I only recently discovered this "trick" (whilst writing writing scipy-stubs, in case anyone might be interested), and I think it could work in this case:
    # models.pyi
    
    class AbstractUser:
        class Meta:
            abstract: ClassVar[bool] = ...
    
    class User:
        class Meta(AbstractUser.Meta):
            # this is allowed (by pyright) -- no need to specify `abstract`
            ...
    

jorenham avatar Oct 16 '24 18:10 jorenham