Refine Site model fields and reorder SiteManager - #3134
ahmedasar00 wants to merge 1 commit into
Conversation
0bd3a7c to
7ac2ef8
Compare
|
|
||
| domain = models.CharField(max_length=100) | ||
| name = models.CharField(max_length=50) | ||
| domain: Any |
There was a problem hiding this comment.
Sorry, why do we change a model field to be Any? What is the original error?
There was a problem hiding this comment.
Sorry, why do we change a model field to be
Any? What is the original error?
I used Any while debugging a stubtest error, but I agree the concrete types are better.
Should I revert them to str / int ?
There was a problem hiding this comment.
I dont think we should change anything here, I'm pretty sure the current state is correctly infered by the mypy plugin
There was a problem hiding this comment.
In my opinion, I find it useful because it prevents forward references and using str and int helps ensure greater accuracy, although I could be wrong.
There was a problem hiding this comment.
Still not convinced it's a good idea, I'm almost sure we loose some strict type checking with mypy when setting attribute for ex.
I don't think this is the way to go, we need to research existing issues and pr on the subject, weight the various options we have and how well they work with typechecker with no plugin (which should be goal, but not at the expense of the mypy setup)
There was a problem hiding this comment.
Thanks for the feedback.
I reopened this PR because I want to find the right solution rather than just close it without understanding the correct approach.
Could you point me to any existing issues or PRs that discussed how the mypy plugin handles Site model field types? That would help me understand the right direction before making further changes.
If you think there's no clear path forward right now, I'm also happy to close this and revisit it later with a better-informed approach.
aaa3b95 to
f6304a8
Compare
ahmedasar00
changed the title
Detailed changes: - Map Site fields (id, domain, name) to concrete types to fix DeferredAttribute errors. - Type SITE_CACHE as dict[int | str, Site] for better precision. - Add missing __str__ method to Site stub. - Remove resolved Site entries from stubtest allowlist.
2502296 to
f29c3fa
Compare
This branch has not been deployed
3 participants
Changes
Anytype hints and replaced them with concrete types:django.contrib.sites.models.Site.domain→strdjango.contrib.sites.models.Site.name→strdjango.contrib.sites.models.Site.id→intSiteManagermethods typing for better accuracy.Notes
SiteManagerdefinition after theSiteclass to avoid potential forward reference issues.