3 ms·

Yep, and my criticism of your suggested approach wasn't intended to be particularly strong by any means. I could definitely be sold on the idea. It just felt li

by j4mie 14y ago

Yep, and my criticism of your suggested approach wasn't intended to be particularly strong by any means. I could definitely be sold on the idea. It just felt like a workaround to a problem that could probably be solved in a nicer way. I think my main objection is that these query methods should conceptually be on the QuerySet, and so defining them on the Manager (the "wrong place") and magically copying them to the QuerySet (the "right place") feels somehow worse than the opposite. I appreciate that you raised the discussion on the mailing list, as it highlights the fact that this is a common problem in big Django codebases. Even pulling something like PassThroughManager into core might work (perhaps with a nicer "manager_with_queryset" API, as suggested by Aramgutang).

zacharyvoase 14y ago
But my problem is that most people wouldn't even think of subclassing QuerySet. When we write methods that operate on collections of things, we typically use @classmethod. Without @classmethod, we'd have to write a custom metaclass (and instruct our class to use that) if we wanted even a single class method on a class. Multiple inheritance would break (or at least be difficult to reason about) when classes defined class methods, because there would have to be both an instance method resolution order and a class MRO. Fortunately Python's built-in `type` provides the descriptor protocol, which allows us to have class methods and instance methods and properties and all these other nice things without having to metaprogram or hack the interpreter. All I'm asking for is a similar (if less ornate) interface for Django models, wherein the methods that operate on collections of things can be defined alongside the methods that operate on individual things, without requiring a knowledge of Manager/QuerySet internals.
- j4mie 14y ago
  > But my problem is that most people wouldn't even think of subclassing QuerySet. I'd argue that's a documentation issue. QuerySet is Django's abstraction of a set of filterable database results. Keeping this conceptually separate from other parts of the ORM, such as the model class, is valuable IMO, and I'm not sure we gain much by trying to hide the details. One of the things I love about Django's philosophy is that it generally provides shortcuts that encapsulate common patterns without obscuring them. There's a fine line between discussing the minutiae of API design and bikeshedding, and I think that anything Django can do to help here is valuable, so I won't push the issue any further. Let's pick it up again on the mailing list or Trac at an appropriate time.
- simonw 14y ago
  A few years ago I was experimenting with exactly this problem, and I came up with an API that worked using an inner class on the model. I can't find the code now, but from memory it looked something like this: class BlogEntry(models.Model, MagicManagerMixin): title = models.CharField(max_length = 128) is_published = models.BooleanField(default = False) class QuerySet(models.QuerySet): def published(self): return self.filter(is_published = True) entries = BlogEntry.objects.published() Where MagicManagerMixin was some scary code that made sure the objects Manager would use the queryset subclass.
legutierr 14y ago
Why not just pass a QuerySet implementation into models.Manager as an optional argument? This would be much cleaner, I think: class QuerySetSubclass(QuerySet): def new_method(self): pass class Model(models.Model): objects = models.Manager(QuerySetSubclass)
- jacobian 14y ago
  This is the approach I'd like to see; I'll be proposing it for addition in Django 1.5.

by j4mie 14y ago

zacharyvoase 14y ago

But my problem is that most people wouldn't even think of subclassing QuerySet. When we write methods that operate on collections of things, we typically use @classmethod. Without @classmethod, we'd have to write a custom metaclass (and instruct our class to use that) if we wanted even a single class method on a class. Multiple inheritance would break (or at least be difficult to reason about) when classes defined class methods, because there would have to be both an instance method resolution order and a class MRO. Fortunately Python's built-in `type` provides the descriptor protocol, which allows us to have class methods and instance methods and properties and all these other nice things without having to metaprogram or hack the interpreter. All I'm asking for is a similar (if less ornate) interface for Django models, wherein the methods that operate on collections of things can be defined alongside the methods that operate on individual things, without requiring a knowledge of Manager/QuerySet internals.

j4mie 14y ago
> But my problem is that most people wouldn't even think of subclassing QuerySet. I'd argue that's a documentation issue. QuerySet is Django's abstraction of a set of filterable database results. Keeping this conceptually separate from other parts of the ORM, such as the model class, is valuable IMO, and I'm not sure we gain much by trying to hide the details. One of the things I love about Django's philosophy is that it generally provides shortcuts that encapsulate common patterns without obscuring them. There's a fine line between discussing the minutiae of API design and bikeshedding, and I think that anything Django can do to help here is valuable, so I won't push the issue any further. Let's pick it up again on the mailing list or Trac at an appropriate time.
simonw 14y ago
A few years ago I was experimenting with exactly this problem, and I came up with an API that worked using an inner class on the model. I can't find the code now, but from memory it looked something like this: class BlogEntry(models.Model, MagicManagerMixin): title = models.CharField(max_length = 128) is_published = models.BooleanField(default = False) class QuerySet(models.QuerySet): def published(self): return self.filter(is_published = True) entries = BlogEntry.objects.published() Where MagicManagerMixin was some scary code that made sure the objects Manager would use the queryset subclass.

legutierr 14y ago

Why not just pass a QuerySet implementation into models.Manager as an optional argument? This would be much cleaner, I think: class QuerySetSubclass(QuerySet): def new_method(self): pass class Model(models.Model): objects = models.Manager(QuerySetSubclass)

jacobian 14y ago
This is the approach I'd like to see; I'll be proposing it for addition in Django 1.5.