Skip to content
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

AbstractUser lacks Meta-attribute #2112

Open
prauscher opened this issue May 2, 2024 · 9 comments · May be fixed by #2168
Open

AbstractUser lacks Meta-attribute #2112

prauscher opened this issue May 2, 2024 · 9 comments · May be fixed by #2168
Labels
bug Something isn't working

Comments

@prauscher
Copy link

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:

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 prauscher added the bug Something isn't working label May 2, 2024
@intgr
Copy link
Collaborator

intgr commented May 4, 2024

@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?

@jorenham
Copy link
Contributor

jorenham commented May 4, 2024

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 🤷🏻

@prauscher
Copy link
Author

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.

@jorenham
Copy link
Contributor

jorenham commented May 5, 2024

@prauscher AbstractBaseUser wasn't mentioned?

@prauscher
Copy link
Author

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.

@jorenham
Copy link
Contributor

@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.

@intgr
Copy link
Collaborator

intgr commented May 12, 2024

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

@steele
Copy link

steele commented Oct 16, 2024

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)

@jorenham
Copy link
Contributor

@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 Remove class Meta from Model and Form class stubs #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`
            ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

4 participants