Instead of trying to dynamically change the manager used for reverse
relations or any manager/queryset (either by getting and then throwing
away the default manager, or some other method), or applying manual
filters to achieve the same result, there is another alternative.

Instead of:

{{{
reporter.article_set.manager('published_articles')
}}}

Do:

{{{
Article.published_articles.filter(reporter=reporter)
}}}

This is probably not ideal, but is still easy to read and should be an
easy work-around in most cases.

Regarding option 1-4 above, only option 2 appeals to me. I prefer my
example above to option 1, and option 3-4 are a bit confusing. But
even for option 2, I think it will fall down when you have multiple
foreign keys to the same model. This is why we have `related_name` on
`ForeignKey` instead of `Manager`, and why it should not be moved to
`related_name` on `Manager`.

Cheers.
Tai.


On Jan 17, 9:12 am, Sebastian Goll <sebastian.g...@gmx.de> wrote:
> Hi all,
>
> I don't know if using the queryset's hypothetical use_manager() method
> would be the right approach to the reverse manager problem. In fact,
> I'm not entirely happy with the current manager() proposal either: it
> feels very much like an after-the-fact change to the default manager.
> That's how it is implemented; first, you retrieve the default manager,
> then you decide that's not what you wanted after all.
>
> I don't think that is how selecting the reverse manager should feel.
> Also, as long as we don't unify managers and querysets entirely, I
> think that whatever returns to us the reverse manager should, in fact,
> return a manager, i.e. we should be able to call all manager methods
> defined on that manager, not just the @queryset ones (given that those
> would be made available in the querysets derived from that manager).
>
> Unfortunately, I'm not sure what would be a better way, or syntax, for
> setting the reverse manager. In #3871, russellm suggested the manager()
> method approach to select which manager is used. To me that feels very
> much like an after-the-fact approach. I also allows you to chain
> manager() calls which doesn't seem very useful at all:
>
> {{{
>   reporter.article_set.manager('published_articles').manager('articles')
>
> }}}
>
> I think we need something different. Just brainstorming, these are some
> thoughts how choosing reverse managers _could_ be implemented; this is
> not yet related to any actual implementation, only some ideas:
>
>   1. use a dict-like approach on the current `related_name` reverse
>     manager, e.g.
>
> {{{
>   reporter.article_set['published_articles']
>
> }}}
>
>     This is not verbose, and probably not intuitive at all; it uses
>     array indexing for something that's not at all an array. However,
>     `article_set` could be thought of as a dict mapping manager names
>     to managers, so maybe it makes at least some sense after all.
>
>   2. add `related_name` to models.Manager as well, e.g.
>
> {{{
> class Article(models.Model):
>     reporter = models.ForeignKey(Reporter, related_name='article_set')
>     ...
>     articles = models.Manager()
>     published_articles = PublishedManager(related_name='published_articles')
>
> }}}
>
>     `reporter.article_set` returns the default manager as before, but
>     now `reporter.published_articles` returns related objects via
>     PublishedManager.
>
>     It would be an error to specify both `related_name` on the
>     ForeignKey and on the default manager. In fact, `related_name` on
>     the ForeignKey could be translated into the `related_name`
>     attribute on the default manager at model import time, keeping
>     `ForeignKey(related_name=(…))` for backwards compatibility as well
>     as for simple models which do not need a manager other than the
>     default models.Manager().
>
>     In fact, the more intuitive and unified way to write the above
>     example (with two managers) would be to move the `related_name`
>     from ForeignKey to the default manager:
>
> {{{
> class Article(models.Model):
>     reporter = models.ForeignKey(Reporter)
>     ...
>     articles = models.Manager(related_name='article_set')
>     published_articles = PublishedManager(related_name='published_articles')
>
> }}}
>
>     Non-default managers without the `related_name` attribute would not
>     be accessible in reverse relations.
>
>   3. make `related_name` on ForeignKey a dict mapping attributes to
>     manager names, e.g.
>
> {{{
> class Article(models.Model):
>     reporter = models.ForeignKey(Reporter, related_name={'article_set': 
> '_default_manager', 'published_articles': 'published_articles'})
>     ...
>     articles = models.Manager()
>     published_articles = PublishedManager()
>
> }}}
>
>     This doesn't seem sensible at all. Lots of boilerplate and overly
>     verbose.
>
>   4. add `reverse_managers` to ForeignKey, specifying which managers
>     (by name) should be made available on the reverse relation, e.g.
>
> {{{
> class Article(models.Model):
>     reporter = models.ForeignKey(Reporter, related_name='article_set', 
> reverse_managers=['published_articles'])
>     ...
>     articles = models.Manager()
>     published_articles = PublishedManager()
>
> }}}
>
>     This seems like a very arbitrary choice. Also, it demonstrates the
>     overlap between `related_name` and `reverse_managers`. Both
>     attributes are responsible for more or less the same thing:
>     defining the name of the reverse-mapping attribute on the related
>     class.
>
> To summarize, I think that selecting custom managers in reverse
> relations is a worthwhile effort. However, it would feel more natural
> if you wouldn't need yet another method for selecting which manager is
> chosen.
>
> In fact, we don't have a special method for selecting non-default
> forward managers, so the same property should hold for non-default
> reverse managers: it all comes down to `Article.articles.(…)` and
> `Article.published_articles.(…)` in the forward relation, so it should
> be equally easy to write `reporter.articles.(…)` and
> `reporter.published_articles.(…)` in the reverse case.
>
> The question remains how this should be defined in the model. Simply
> sticking _all_ reverse descriptors in the related class doesn't seem
> like a sensible thing to do; this was the original approach in #3871
> and was dismissed by russellm.
>
> That said, I lean towards explicitly naming the managers which should
> be made available, a.k.a. idea #2 from my list above.
>
> This way, you also only had to define the backwards name once and could
> use it for all reverse relations referring to that model; which makes
> sense somewhat as it's still the same related model in either case,
> with the same managers (contrast this to adding an attribute to the
> ForeignKey that only influences the relation between two particular
> models). On the other hand, this could easily introduce name clashes
> between forward and reverse attributes.
>
> Any thoughts?
>
> Sebastian.
>
> On Sat, 14 Jan 2012 22:15:33 +0200
>
>
>
>
>
>
>
> Kääriäinen Anssi <anssi.kaariai...@thl.fi> wrote:
> > Just a quick thought: you should check out the work done for allowing use 
> > of manager methods in querysets (thread "RFC: query methods"). It seems 
> > that work does have some overlap with this feature. The patch for #3871 
> > implements .manager('manager_name') for reverse relation managers, and 
> > there was some discussion for allowing .use_manager('manager_name') for 
> > querymethods. .use_manager() is not going to be in the query methods patch. 
> > I haven't looked #3871 in detail, but maybe the work done for query methods 
> > would make the #3871 patch easier to implement?
>
> > The idea would be to issue: .use_manager(wanted_manager).all() in the 
> > .manager() method. The first method call would change the base manager to 
> > use, the second (.all) call would make it return a queryset, so that you 
> > would not have the .clear and .remove methods available. This might be a 
> > stupid idea, but maybe worth a try? The .use_manager() call would not need 
> > to exist on queryset level.
>
> > 1.4 is feature frozen if I am not mistaken, so this would be 1.5 stuff.
>
> >  - Anssi
> > ________________________________________
> > From: django-developers@googlegroups.com 
> > [django-developers@googlegroups.com] On Behalf Of Sebastian Goll 
> > [sebastian.g...@gmx.de]
> > Sent: Saturday, January 14, 2012 21:35
> > To: django-developers@googlegroups.com
> > Subject: Re: Custom managers in reverse relations
>
> > Hi all,
>
> > My latest post to the list seems to have been lost in the pre-Christmas
> > storm. Sorry for that!
>
> > The issue of picking which custom manager is used in resolving reverse
> > relations still stands. Let my give you an example why this is useful:
>
> > {{{
> > class Reporter(models.Model):
> >     ...
>
> > class Article(models.Model):
> >     reporter = models.ForeignKey(Reporter)
> >     ...
> >     articles = models.Manager()
> >     published_articles = PublishedManager()
> > }}}
>
> > We put some thought into designing PublishedManager. Maybe it needs to
> > do some things in addition to simply checking a flag, who knows. The
> > thing is: right now, we simply cannot make use of this manager when
> > looking up a reporter's articles: with `reporter.article_set` we
> > always get _all_ articles. [1]
>
> > Now we have two options: doing the filtering manually, on the returned
> > queryset, or specify that we want to use PublishedManager, accessible
> > through the `published_articles` attribute of the Article class.
>
> > The latter is implemented by the patches in ticket #3871:
>
> >  https://code.djangoproject.com/ticket/3871
>
> > Does this seem like a good idea? Should it even be possible to specify
> > which custom manager is used for reverse relations? Or, am I missing
> > something and this is already possible in some other way?
>
> > Since I'm looking forward to seeing this implementation in Django 1.4,
> > I ask for your input on the matter.
>
> > Thanks!
> > Sebastian.
>
> > [1] In fact, that's not entirely true: we get whatever is returned by
> > the _default_ manager of the Article class. This seems like an
> > arbitrary choice: it's not a "plain" manager that always returns all
> > related instances, it's whatever we picked as the default manager.
>
> > On Fri, 23 Dec 2011 21:56:24 +0100
> > Sebastian Goll <sebastian.g...@gmx.de> wrote:
>
> > > Hi all,
>
> > > I'd like to draw your attention to long-open ticket #3871 [1].
>
> > > The idea is to let ORM users choose which custom manager to use for 
> > > reverse "many" relations, i.e. reverse foreign key (…_set) as well as 
> > > forward and reverse many-to-many relations.
>
> > > There are several proposed patches to this ticket, the latest was added 
> > > by me a week ago. The current implementation adds a "manager()" method to 
> > > the reverse manager which allows you to pick a manager different from the 
> > > default one on the related model. All changes are entirely 
> > > backwards-compatible – if you don't call the "manager()" method, 
> > > everything is as before, i.e. the default manager is used to look up 
> > > related model instances.
>
> > > During my review of the previous patch I found that it doesn't apply 
> > > cleanly to trunk, as well as some concerns with regard to the
>
> ...
>
> read more »

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Reply via email to