Got interested in this after a referral from the tricket tracker. Couldn't
this specific case be solved with a simple change to __getattr__ on
QuerySet?
def __getattr__(self, attr):
try:
qmethod = getattr(self.manager, attr).im_func
is_querymethod = qmethod.is_querymethod
except AttributeError, exc:
raise AttributeError(attr)
if is_querymethod:
- return curry(qmethod, self)
+ return curry(qmethod, self.manager)
raise AttributeError(attr)
The only issue I see here is that it assumes self.manager is always defined
so this would break if a QuerySet was instantiated outside of
Manager.get_query_set(). But really, it doesn't make sense to add the
@querymethod decorator on QuerySet functions anyway.
- Michael
Le jeudi 12 janvier 2012 09:05:57 UTC-5, Anssi Kääriäinen a écrit :
>
> On Jan 12, 2:24 pm, Zachary Voase <[email protected]> wrote:
> > I strongly feel that switching around managers like that would be an
> > example of overengineering.
>
> I tend to over-engineer things. It is easy to add the manager
> switching later on if needed. And it is easy to switch managers by
> just changing qs.manager if really needed. So, I agree on leaving that
> feature out.
>
> > Here's the change I propose:
> https://github.com/zacharyvoase/django/compare/master...feature-query...
> >
> > It's very light for now, I'm not sure if there are other corner cases
> > where it breaks, but it's the simplest thing that could possibly work,
> > and it's conceptually very easy to understand.
>
> And in the implementation I disagree. The implementation magically
> changes what self references in querymethods. This will break any code
> that wants to reference self in manager. For example:
>
> def get_id_list_by_raw_sql(self, some_params):
> return cursor.execute("select if from sometable where someparam =
> %s", (some_param,))
>
> @querymethod
> def list_interesting_subjects(self, some_params):
> self.filter(id__in=self.get_id_list_by_raw_sql(some_params))
>
> Now, the above will work just fine if called through manager, but it
> will not work if called through queryset, because self suddenly refers
> to the qs, not to the manager. Depending how it is called. So, I would
> suggest an approach like in my version, where self is always the
> manager, and you get the queryset by self.get_query_set() (or maybe
> with self.base_qs, which is a property returning either
> self.get_query_set() or the queryset called from).
>
> The main reason why I think it is a bad idea to "transfer" the method
> to the qs is that if you do that, you will confuse people (what does
> self refer to?), forbid some usecases, and gain exactly nothing by
> doing that. And that can not be changed later on. Why not just let the
> method be always bound to the manager instance? That is much cleaner.
> This all assuming I actually understand what the patch does. Can't
> test it currently...
>
> Am I missing something that is gained by currying the method to
> queryset instead of letting it be bound to the manager?
>
> Still another idea is to somehow patch the parameters to the method in
> querymethod, so that the queryset to chain to would be passed as a
> parameter to the querymethod. It would either be self.get_query_set()
> or the queryset called through.
>
> - Anssi
Le jeudi 12 janvier 2012 09:05:57 UTC-5, Anssi Kääriäinen a écrit :
>
> On Jan 12, 2:24 pm, Zachary Voase <[email protected]> wrote:
> > I strongly feel that switching around managers like that would be an
> > example of overengineering.
>
> I tend to over-engineer things. It is easy to add the manager
> switching later on if needed. And it is easy to switch managers by
> just changing qs.manager if really needed. So, I agree on leaving that
> feature out.
>
> > Here's the change I propose:
> https://github.com/zacharyvoase/django/compare/master...feature-query...
> >
> > It's very light for now, I'm not sure if there are other corner cases
> > where it breaks, but it's the simplest thing that could possibly work,
> > and it's conceptually very easy to understand.
>
> And in the implementation I disagree. The implementation magically
> changes what self references in querymethods. This will break any code
> that wants to reference self in manager. For example:
>
> def get_id_list_by_raw_sql(self, some_params):
> return cursor.execute("select if from sometable where someparam =
> %s", (some_param,))
>
> @querymethod
> def list_interesting_subjects(self, some_params):
> self.filter(id__in=self.get_id_list_by_raw_sql(some_params))
>
> Now, the above will work just fine if called through manager, but it
> will not work if called through queryset, because self suddenly refers
> to the qs, not to the manager. Depending how it is called. So, I would
> suggest an approach like in my version, where self is always the
> manager, and you get the queryset by self.get_query_set() (or maybe
> with self.base_qs, which is a property returning either
> self.get_query_set() or the queryset called from).
>
> The main reason why I think it is a bad idea to "transfer" the method
> to the qs is that if you do that, you will confuse people (what does
> self refer to?), forbid some usecases, and gain exactly nothing by
> doing that. And that can not be changed later on. Why not just let the
> method be always bound to the manager instance? That is much cleaner.
> This all assuming I actually understand what the patch does. Can't
> test it currently...
>
> Am I missing something that is gained by currying the method to
> queryset instead of letting it be bound to the manager?
>
> Still another idea is to somehow patch the parameters to the method in
> querymethod, so that the queryset to chain to would be passed as a
> parameter to the querymethod. It would either be self.get_query_set()
> or the queryset called through.
>
> - Anssi
Le jeudi 12 janvier 2012 09:05:57 UTC-5, Anssi Kääriäinen a écrit :
>
> On Jan 12, 2:24 pm, Zachary Voase <[email protected]> wrote:
> > I strongly feel that switching around managers like that would be an
> > example of overengineering.
>
> I tend to over-engineer things. It is easy to add the manager
> switching later on if needed. And it is easy to switch managers by
> just changing qs.manager if really needed. So, I agree on leaving that
> feature out.
>
> > Here's the change I propose:
> https://github.com/zacharyvoase/django/compare/master...feature-query...
> >
> > It's very light for now, I'm not sure if there are other corner cases
> > where it breaks, but it's the simplest thing that could possibly work,
> > and it's conceptually very easy to understand.
>
> And in the implementation I disagree. The implementation magically
> changes what self references in querymethods. This will break any code
> that wants to reference self in manager. For example:
>
> def get_id_list_by_raw_sql(self, some_params):
> return cursor.execute("select if from sometable where someparam =
> %s", (some_param,))
>
> @querymethod
> def list_interesting_subjects(self, some_params):
> self.filter(id__in=self.get_id_list_by_raw_sql(some_params))
>
> Now, the above will work just fine if called through manager, but it
> will not work if called through queryset, because self suddenly refers
> to the qs, not to the manager. Depending how it is called. So, I would
> suggest an approach like in my version, where self is always the
> manager, and you get the queryset by self.get_query_set() (or maybe
> with self.base_qs, which is a property returning either
> self.get_query_set() or the queryset called from).
>
> The main reason why I think it is a bad idea to "transfer" the method
> to the qs is that if you do that, you will confuse people (what does
> self refer to?), forbid some usecases, and gain exactly nothing by
> doing that. And that can not be changed later on. Why not just let the
> method be always bound to the manager instance? That is much cleaner.
> This all assuming I actually understand what the patch does. Can't
> test it currently...
>
> Am I missing something that is gained by currying the method to
> queryset instead of letting it be bound to the manager?
>
> Still another idea is to somehow patch the parameters to the method in
> querymethod, so that the queryset to chain to would be passed as a
> parameter to the querymethod. It would either be self.get_query_set()
> or the queryset called through.
>
> - Anssi
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/django-developers/-/3fyLDAVyDrIJ.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
