Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  master
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  1|
-+-
Changes (by Anssi Kääriäinen ):

 * status:  reopened => closed
 * resolution:   => fixed


Comment:

 In [changeset:"69a46c5ca7d4e6819096af88cd8d51174efd46df"]:
 {{{
 #!CommitTicketReference repository=""
 revision="69a46c5ca7d4e6819096af88cd8d51174efd46df"
 Tests for various emptyqs tickets

 The tickets are either about different signature between qs.none() and
 qs or problems with subclass types (either EmptyQS overrided the custom
 qs class, or EmptyQS was overridden by another class - values() did
 this).

 Fixed #15959, fixed #17271, fixed #17712, fixed #19426
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  reopened
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  1|
-+-

Comment (by lukeplant):

 OK, sorry, I didn't look at the other patch closely.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  reopened
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  1|
-+-
Changes (by carljm):

 * status:  closed => reopened
 * resolution:  duplicate =>
 * stage:  Unreviewed => Accepted


Comment:

 Replying to [comment:9 lukeplant]:
 > Special casing the `none()` method doesn't seem very compelling compared
 to #17270 which addresses the larger problem, therefore closing this bug
 as a duplicate of #17270.

 I don't agree with this resolution. I am not at all convinced that the
 technique in #17270 is a candidate for core (although it can be nice as a
 third-party module), as it adds a bunch of extra runtime code execution
 and implicit magical behavior to every single ORM call. And it's not
 really addressing the same problem at all: it's addressing the difference
 between a `Manager` and a `QuerySet`, not the difference between a
 `QuerySet` and an `EmptyQuerySet`. If you look at the patch there, it only
 fixes this problem too because it special-cases `.none()` and
 `EmptyQuerySet` every bit as much as a patch for this would. In other
 words, these are two separate issues, it just happens that #17270
 conflates fixes for both of them into a single patch.

 In any case, `.none()` is already special-cased by the existence of
 `EmptyQuerySet` - this bug is just looking for a remedy to the
 consequences of that existing special-case. I think it's a legitimate, and
 separate, bug.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:  duplicate
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by andreypopp):

 Replying to [comment:8 Wojciech Banaś ]:
 > Your comments were very useful. Have combined your ideas with me, and I
 received a very short and working code ([[ticket:17270]], see attachment
 ''manager.3.py'').
 >
 > Do you think that this solution would be good now?

 It still generates new class at runtime and performs method mangling which
 can be quite expensive in terms of performance. As I've already said, I
 suggest to merge `Manager` and `QuerySet` functionality into single class
 or eliminate one of them from public API, probably `Manager` is the best
 candidate for this.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:  duplicate
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by lukeplant):

 * status:  new => closed
 * resolution:   => duplicate


Comment:

 Special casing the `none()` method doesn't seem very compelling compared
 to #17270 which addresses the larger problem, therefore closing this bug
 as a duplicate of #17270.

 Thanks.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by Wojciech Banaś ):

 Replying to [comment:7 andreypopp]:
 > Yep, indeed, I actually wondered if I could provide all methods on
 `Manager` and `QuerySet` simultaneously and haven't found any solutions
 which look good. Your patch implements this correctly but I think it:
 >  * suffers from performance degradation(very much code are executed
 during `get_query_set()` call which isn't rare call, I think)
 >  * doesn't work if `Manager` implements `__getattr__`
 > There's also a strange spot if `QuerySet` already has some methods — you
 decided not to override such methods and I think it's good decision while
 it also makes some implicit assumptions which can confuse users and create
 new "special case" in documentation.
 >
 > For me the better and the bigger spot to touch is to completely merge
 `Manager` and `QuerySet` classes into one unit or just expose only on them
 in API.

 Your comments were very useful. Have combined your ideas with me, and I
 received a very short and working code ([[ticket:17270]], see attachment
 ''manager.3.py'').

 Do you think that this solution would be good now?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by andreypopp):

 Replying to [comment:6 Wojciech Banaś ]:
 > The same you can get by using my patch, and filters are where they
 should, in the manager.
 >
 > See my examples in this ticket [[ticket:17270]]

 Yep, indeed, I actually wondered if I could provide all methods on
 `Manager` and `QuerySet` simultaneously and haven't found any solutions
 which look good. Your patch implements this correctly but I think it:
  * suffers from performance degradation(very much code are executed during
 `get_query_set()` call which isn't rare call, I think)
  * doesn't work if `Manager` implements `__getattr__`
 There's also a strange spot if `QuerySet` already has some methods — you
 decided not to override such methods and I think it's good decision while
 it also makes some implicit assumptions which can confuse users and create
 new "special case" in documentation.

 For me the better and the bigger spot to touch is to completely merge
 `Manager` and `QuerySet` classes into one unit or just expose only on them
 in API.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by Wojciech Banaś ):

 Replying to [comment:5 andreypopp]:
 > Replying to [comment:4 Wojciech Banaś ]:
 > > I tested also a problem with the method of none () in my
 solution([[ticket:17270]]), and this problem does NOT exist there.
 > >
 > > So I think the filters should be located just in manager and not in
 the QuerySet.
 >
 > I like to define methods on `QuerySet` because it allows chain several
 filters at once:
 > {{{
 > users = User.objects.have_permission("some
 permission").is_staff().have_new_messages()
 > }}}
 > and so on...
 The same you can get by using my patch, and filters are where they should,
 in the manager.

 See my examples in this ticket [[ticket:17270]]

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by andreypopp):

 Replying to [comment:4 Wojciech Banaś ]:
 > I tested also a problem with the method of none () in my
 solution([[ticket:17270]]), and this problem does NOT exist there.
 >
 > So I think the filters should be located just in manager and not in the
 QuerySet.

 I like to define methods on `QuerySet` because it allows chain several
 filters at once:
 {{{
 users = User.objects.have_permission("some
 permission").is_staff().have_new_messages()
 }}}
 and so on...

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by Wojciech Banaś ):

 * cc: fizista@… (added)


Comment:

 Replying to [comment:3 Wojciech Banaś ]:
 > The same does my patch ([[ticket:17270]]), but the filter is added to
 the manager and not in queries.

 I tested also a problem with the method of none () in my
 solution([[ticket:17270]]), and this problem does NOT exist there.

 So I think the filters should be located just in manager and not in the
 QuerySet.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by Wojciech Banaś ):

 The same does my patch ([[ticket:17270]]), but the filter is added to the
 manager and not in queries.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by andreypopp):

 * cc: 8mayday@… (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
-+-
 Reporter:  andreypopp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by andreypopp):

 * needs_better_patch:   => 0
 * type:  Bug => New feature
 * needs_tests:   => 0
 * needs_docs:   => 0


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

[Django] #17271: QuerySet.none() doesn't work well with subclasses of QuerySet

[Django]

#17271: QuerySet.none() doesn't work well with subclasses of QuerySet
--+
 Reporter:  andreypopp|  Owner:  nobody
 Type:  Bug   | Status:  new
Component:  Database layer (models, ORM)  |Version:  SVN
 Severity:  Normal|   Keywords:
 Triage Stage:  Unreviewed|  Has patch:  1
Easy pickings:  1 |  UI/UX:  0
--+
 I use `QuerySet` subclassing to allow building complex composable queries
 out of smaller ones:
 {{{
 class MyQuerySet(QuerySet):
   def allowed(self, user):
 return self.filter(permissions__in=user.permissions)

   def have_feature(feature_name):
 return self.filter(feature__name=feature_name)
 ...

 objs = Obj.objects.allowed(user).have_feature("somefeature")
 }}}

 Sometimes I want to return `EmptyQuerySet` by calling `none()` to provide
 "fast-path" for some conditions, but unfortunately `EmptyQuerySet` doesn't
 have my custom methods, so call chain breaks after returning it:

 {{{
 class MyQuerySet(QuerySet):
   def allowed(self, user):
 if user.is_anonymous:
   return self.none()
 return self.filter(permissions__in=user.permissions)

   def have_feature(feature_name):
 return self.filter(feature__name=feature_name)
 ...

 objs = Obj.objects.allowed(user).have_feature("some feature")
 # AttributeError: EmptyQuerySet doesn't have have_feature attribute ...
 }}}

 I've attached a patch which instruments `EmptyQuerySet` subclass for each
 particular `QuerySet` implementation and returns its instance as a result
 of `none()` method call. I've also refactored code in `Manager.none()` to
 reuse existing code in `QuerySet.none()` and to parametrize `Manager()`
 constructor over `QuerySet` implementation to use.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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