Skip to content

Type admin site as DefaultAdminSite inheriting from Adminsite #3296

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.

Sign up for GitHub

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

Open
ahmedasar00 wants to merge 1 commit into
base: master
Choose a base branch
from
+3 −4
Open

Type admin site as DefaultAdminSite inheriting from Adminsite #3296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions django-stubs/contrib/admin/sites.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ from django.http.request import HttpRequest
from django.http.response import HttpResponse, HttpResponseBase
from django.template.response import TemplateResponse
from django.urls import URLPattern, URLResolver
from django.utils.functional import LazyObject, _StrOrPromise
from django.utils.functional import _StrOrPromise

all_sites: WeakSet[AdminSite]

Expand Down Expand Up @@ -81,6 +81,6 @@ class AdminSite:
def catch_all_view(self, request: HttpRequest, url: str) -> HttpResponse: ...
def get_log_entries(self, request: HttpRequest) -> QuerySet[LogEntry]: ...

class DefaultAdminSite(LazyObject[AdminSite]): ...
class DefaultAdminSite(AdminSite): ...

site: AdminSite
Copy link
Copy Markdown
Contributor

@UnknownPlatypus UnknownPlatypus Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is tricky but I don't think this is a correct fix.

The LazyObject make this site behaves like an AdminSite object. You can try it yourself

>>> from django.contrib.admin.sites import site
>>> site
AdminSite(name='admin')

Copy link
Copy Markdown
Contributor Author

@ahmedasar00 ahmedasar00 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I agree it's tricky, I understand that DefaultAdminSite proxies to DefaultAdminSite via LazyObject, not through actual inheritance.!

What would be the correct approach here?

Copy link
Copy Markdown
Contributor

@UnknownPlatypus UnknownPlatypus Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the stubtest error exactly ?

We probably will just have to mive this allowlist entry to the main allowlist.txt with a comment, possibly next to similar entries if there are any

Copy link
Copy Markdown
Contributor Author

@ahmedasar00 ahmedasar00 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error: django.contrib.admin.site variable differs from runtime type
django.contrib.admin.sites.DefaultAdminSite
Stub: in file django-stubs/contrib/admin/__init__.pyi:86
django.contrib.admin.sites.AdminSite
Runtime:
AdminSite(name='admin')

And the same for django.contrib.gis.admin.site.

I can update this PR to move the entry to allowlist.txt with a comment instead, if you'd prefer that approach.

Copy link
Copy Markdown
Contributor

@UnknownPlatypus UnknownPlatypus Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be to get to the bottom of this, it's either a bug in stubtest or something wrong with how we type LazyFunc or use it here

Should it be that maybe ?

class DefaultAdminSite(AdminSite): ...
site: LazyObject[AdminSite]

but site: DefaultAdminSite will cause false positives for pretty much every attribute access that are expected on AdminSite but not present here no ?

Copy link
Copy Markdown
Contributor Author

@ahmedasar00 ahmedasar00 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting finding: type(site) returns DefaultAdminSite, but site.__class__ returns AdminSite because LazyObject.__getattr__ proxies __class__ to the wrapped object. That's why isinstance(site, AdminSite) is True even though issubclass(DefaultAdminSite, AdminSite) is False.

So the issue seems to be that stubtest uses type(site) and sees DefaultAdminSite, while the stub says AdminSite. This might be a stubtest limitation, it doesn't account for LazyObject's __class__ proxying behavior.

Should I investigate how LazyObject is typed to see if there's a way to make this work properly?

Copy link
Copy Markdown
Contributor

@UnknownPlatypus UnknownPlatypus Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the issue seems to be that stubtest uses type(site) and sees DefaultAdminSite, while the stub says AdminSite. This might be a stubtest limitation, it doesn't account for LazyObject's class proxying behavior.

@sobolevn Shoul we do something about it ? Is it a stubtest limitation with thes proxy objects ?

site: DefaultAdminSite
1 change: 0 additions & 1 deletion scripts/stubtest/allowlist_todo.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ django.contrib.admin.models.LogEntry.object_id
django.contrib.admin.models.LogEntry.object_repr
django.contrib.admin.models.LogEntry.user
django.contrib.admin.models.LogEntry.user_id
django.contrib.admin.sites.site
django.contrib.auth.admin.UserAdmin.fieldsets
django.contrib.auth.base_user.AbstractBaseUser.last_login
django.contrib.auth.base_user.AbstractBaseUser.password
Expand Down
Loading