Make plugin handle explicitly declared reverse descriptors for FKs #2230
Conversation
If a reverse descriptor was explicitly declared on a model, the plugin skipped to create a more specific subclass for its related manager.
| from django.db.models.fields.related_descriptors import ReverseManyToOneDescriptor | ||
|
|
||
| class Other(models.Model): | ||
| explicit_descriptor: ClassVar[ReverseManyToOneDescriptor["MyModel"]] |
There was a problem hiding this comment.
When is this useful? Isn't it better to just write = ReverseManyToOneDescriptor(...)?
There was a problem hiding this comment.
When you don't use mypy and also don't want to override anything that Django does during runtime.
There was a problem hiding this comment.
Or rather: when you don't use our mypy plugin
There was a problem hiding this comment.
Maybe we then need a assert_type test for this instead?
There was a problem hiding this comment.
I suppose we could, but we'd have to make it assert to a different type depending on if we're running with mypy or pyright. Since the plugin creates a more specific type. It didn't feel very useful to be honest. But I can add it if you find that it could be useful
The test here was merely for the plugin, it failed initially so it was some form of regression. I wanted to ensure that the more specific type was set even though an explicit descriptor was declared. I discovered the plugin had an early exit that stopped the descriptor refinement
There was a problem hiding this comment.
Actually, thinking twice about it. I don't think that mypy would output anything different, since it currently requires the runtime to output a more specialised type
There was a problem hiding this comment.
FWIW I added an assert_type test
|
What do you think? Does this look fine? |
If a reverse descriptor was explicitly declared on a model, the plugin skipped to create a more specific subclass for its related manager.
Refs: #2227