#24863 closed New feature (wontfix)
Make `django.db.models.Manager.from_queryset` copy over properties and not just methods
| Reported by: | Ram Rachum | Owned by: | Furkan Karataş |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Loic Bistuer, Marc Tamlyn | Triage Stage: | Unreviewed |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
When I define a QuerySet subclass, I like defining not only methods but properties on it.
The function django.db.models.Manager.from_queryset was introduced so we could succinctly create a Manager subclass that gets the same functionality that we defined on our QuerySet subclass. It copies over all the methods defind in the QuerySet class to the Manager class. But it doesn't copy properties, so they remain inaccessible from the Manager class.
I want django.db.models.Manager.from_queryset to copy over the properties as well.
Change History (13)
comment:1 by , 10 years ago
| Cc: | added |
|---|
comment:2 by , 10 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 10 years ago
| Cc: | added |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
Queryset at the moment has only two properties, both of which make no sense on the manager (ordered and db).
I'm tentatively accepting this ticket, but I would like someone to explain a concrete use case.
comment:4 by , 10 years ago
I assume the OP is referring to dynamic properties (e.g. using @property or @cached_property - so basically methods but called implicitly), as that's the only use case for this I can imagine. Copying over static attributes doesn't make much sense to me.
I'm not strongly opposed to this, but if the implementation turns out to be complex or difficult, I'd be perfectly happy to close it wontfix and say "just use methods instead."
comment:5 by , 10 years ago
Carl: Yes, that's my intention.
mjtamlyn: Okay, here's a concrete use case. I have a model CreditCard and a corresponding queryset CreditCardQuerySet. I want to have a property expired on CreditCardQuerySet so I could filter to expired credit cards like CreditCard.objects.expired. (Assuming there isn't a straightforward query to do that.) I could do it as a method but I always prefer using a property when there aren't any arguments.
comment:6 by , 10 years ago
I have no idea how we'd be able to only take those properties and no attributes or other properties, except for requiring an attribute on the property. But I think just using a function without any arguments is just fine.
comment:7 by , 10 years ago
I'm not really understanding what the implementation problem is. If you're going over all the members of the QuerySet class, you can see which are properties and then copy them. So what's the problem?
comment:8 by , 10 years ago
Stylistically, I'd say a property is wrong for that use case, I don't like properties which do round trips to the database as when reading the code it looks like an attribute. That said...
The implementation is straightforwards, we can use https://docs.python.org/3/library/inspect.html#inspect.isdatadescriptor
comment:9 by , 10 years ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:10 by , 10 years ago
Would this solve #25201? That is, use_for_related_fields could be set on the QuerySet and then copied to the manager when using as_manager()? Would it be better to have something like .as_manager(use_for_related_fields=True)?
comment:11 by , 2 years ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:12 by , 2 years ago
| Has patch: | set |
|---|
PR is opened: https://github.com/django/django/pull/16919
comment:13 by , 2 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | assigned → closed |
| Triage Stage: | Accepted → Unreviewed |
I agree with Lily, Marc and Carl weren't convinced either. Using properties in Queryset to perform queries is confusing, it gets even more puzzling when we realize that some of them (e.g. ordered, query) have to be omitted in a potential implementation. IMO, it's not worth the additional complexity.
Loic, what do you think about this?