Manager with generic QuerySet using TypeVar defaults - #2776
UnknownPlatypus wants to merge 15 commits into
Conversation
The `self.manager_info` can trigger a deferral pass, leaving an incomplete model TypeInfo. Splitting this in 2 makes it idempotent
I've done this in the same pr because now that we infer more correctly the queryset type, we were infering incomplete types more often
Need to comment on the related issue to ask for generic params to be provided
| use_in_migrations: bool | ||
| name: str | ||
| model: type[_T] | ||
| _queryset_class: type[_QS] |
There was a problem hiding this comment.
I needed to expose this symbol because I use it in the plugin.
If we don't want that, I can also add a type-ignore in the plugin code but I think it's fine to have it because it's a core component of a manager and very unlikely to disappear
Manager with generic QuerySet using TypeVar defaults
|
@UnknownPlatypus do you want eyes on this? I had issues with annotations that ended up with me writing this test, maybe this branch could end up resolving those too |
|
@rtpg I plan on rebasing this pr latter this week now that most of the blocking issue mentioned in the description are resolved. If you have some time, it would be great to resolve the annotations in custom queryset issue. Without that, the current state of this PR will cause a lot of issues for such cases that were previously ignored I'll integrate the test you proposed in this MR, I think it will work |
I have made things!
This is redo for #1270 using
TypeVardefaults to implement the change in a non-breaking way.This is not completely ready, I opening it to get some feedback and because there are still a few issues to address.
The PR currently passes tests but I've tried it on my work codebase and some related issues have resurfaced (not caused by this PR, but by the fact that a lot of places previously ignored are now checked)
Most notably:
.annotate()in custom queryset method looses typeI had to do the same re-parametrization trick we do for-> was done in Reparametrize implicit generic QuerySet subclasses #3217Manager(cf Reparametrize managers without explicit type parameters #1169) which causes exactly the same kind of issue with overrides ofmodels.Manager.get_querysetwhich are rather common because documentedAlso Prefetch's to_attr raises "Model" has no attribute "prefetched_field" #795 was more present-- Fixed in Initial support forto_attrinference inPrefetchcalls #2779)I think this is step in the right direction but I'm a bit afraid it might cause some churn if we don't address the related issues first.
Todos
django.contrib.auth.models.Permissionbreaks thefrom_querysetmachinery #287 ?Related issues
Also fixes a bunch of related issues, I've added regression tests for them: