Re: Implement QuerySet.__contains__?

2020-06-05 Thread Shai Berger
On Fri, 5 Jun 2020 13:58:47 +0200 Johan Schiff wrote: > I still think the wrong call was made here [...] > *(Assuming queryset should be evaluated is probably correct in > most cases, but sometimes adds a big performance hit when the code > goes into production and the dataset grows - code that

Re: Implement QuerySet.__contains__?

2020-06-05 Thread Tim Graham
(You can reopen https://code.djangoproject.com/ticket/24141 rather than creating a new ticket.) On Friday, June 5, 2020 at 9:06:48 AM UTC-4, Aymeric Augustin wrote: > > If people agree QuerySet.contains() should be added, how do we move > forward? > > > Yes, if there's no major new argument in

Re: Implement QuerySet.__contains__?

2020-06-05 Thread Aymeric Augustin
> If people agree QuerySet.contains() should be added, how do we move forward? Yes, if there's no major new argument in a couple days, we can assume consensus on adding .contains(). * Create a ticket in Trac. Point to this discussion and to the previous ticket where .contains() was suggested.

Re: Implement QuerySet.__contains__?

2020-06-05 Thread Johan Schiff
Shai does make a good point about changing a well documented behaviour. That argument wins me over. Adding .contains() and updating the documentation goes a long way to make it easier for developers. Best case would be that Dajngo does not make assumptions about what the developer wants, but to

Re: Implement QuerySet.__contains__?

2020-06-05 Thread אורי
Hi, I'm thinking, maybe instead of: if obj in queryset: Users should write: if obj in list(queryset): Which I understand is already working? Why does the first line (without list()) should work anyway? And if users want performance, why not write: if queryset.filter(pk=obj.pk).exists():

Re: Implement QuerySet.__contains__?

2020-06-05 Thread Adam Johnson
I'm with Shai, against changing bool() and len() behaviour. I think the "fetch the whole queryset" behaviour is normally helpful for beginners, who write things like: if qs: for x in qs: ... # really common for anyone new to Python: for i in len(qs): do_something(qs[i]) We have

Re: Implement QuerySet.__contains__?

2020-06-05 Thread Shai Berger
On Tue, 2 Jun 2020 20:57:54 +0200 Aymeric Augustin wrote: > > We're talking about a trade-off between preserving optimisations in > existing code bases and expertise of advanced users versus doing the > right thing by default for less experienced users. I disagree. The suggestion is to make

Re: Implement QuerySet.__contains__?

2020-06-03 Thread Johan Schiff
To answer my own question: No, I wasn't doing it correctly. I should have done a sanity check before posting. New timeit code and results at bottom, now using a smaller dataset (~900 objects). Notable: - An .exists() query is about 1/100 the time of full fetch in this case. This

Re: Implement QuerySet.__contains__?

2020-06-03 Thread Johan Schiff
Thanks for great info. First, I'm leaning towards Aymeric's proposition here. I do recognize that there is a lot to consider. This seems to be important: 1. Developers must be able to explicitly choose methods to optimize for their environment. (Considering database latency, dataset size,

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Roger Gammans
On Tue, 2020-06-02 at 11:31 -0700, Javier Buzzi wrote: > ps @roger > > >>> timeit.timeit('m.GL.objects.filter(pk=x.pk)', setup='import > myapp.models as m;x = m.GL.objects.all()[324]', number=100) > 0.05818330496549606 > > is not doing anything, add a `.exists()` or `len(..)` or something to >

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Aymeric Augustin
o/django/pull/3906 - Efficient QuerySet.__contains__ > (closed as wontfix due to the behavior change) Thanks. Your memory is amazing :-) Since the issue keeps coming back, I think we should do one of the following: 1. make simultaneous and consistent changes to __nonzero__, __len__, and __contains__ to

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Javier Buzzi
My 2 cents, I think @johan's suggestion makes sense. if obj in queryset: It's very pythonic. it should do what __len__ does and cache it, if you want the single quick db query you can always use exists(). ps @roger >>> timeit.timeit('m.GL.objects.filter(pk=x.pk)', setup='import myapp.models

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Tim Graham
And here's some past discussion: https://code.djangoproject.com/ticket/24141 - contains() method for QuerySets (closed as needsinfo due to no mailing list discussion to find a consensus) https://github.com/django/django/pull/3906 - Efficient QuerySet.__contains__ (closed as wontfix due

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Roger Gammans
96549606>>> timeit.timeit('x in qs', setup='import myapp.models as m;qs = m.GL.objects.all(); x=qs[324]', number=100)1.5688817161135375 On Tue, 2020-06-02 at 05:53 -0700, Tim Graham wrote: > It may help to know that QuerySet.__contains__() was implemented > until Django 1.6 when chu

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Tim Graham
It may help to know that QuerySet.__contains__() was implemented until Django 1.6 when chunked reads were removed from QuerySet iteration: https://github.com/django/django/commit/70679243d1786e03557c28929f9762a119e3ac14 On Tuesday, June 2, 2020 at 7:43:25 AM UTC-4, Aymeric Augustin wrote

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Aymeric Augustin
> On 2 Jun 2020, at 13:30, Florian Apolloner wrote: > > On Tuesday, June 2, 2020 at 11:28:34 AM UTC+2, Adam Johnson wrote: > If you already fetched the queryset, `if obj in queryset` will make a new > database query > > I don't see why we couldn't implement __contains__ to do a walk through >

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Florian Apolloner
On Tuesday, June 2, 2020 at 11:28:34 AM UTC+2, Adam Johnson wrote: > > If you already fetched the queryset, `if obj in queryset` will make a new >> database query > > > I don't see why we couldn't implement __contains__ to do a walk through > _result_cache if it has been fetched? > we are doing

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Adam Johnson
> > If you already fetched the queryset, `if obj in queryset` will make a new > database query I don't see why we couldn't implement __contains__ to do a walk through _result_cache if it has been fetched? On Tue, 2 Jun 2020 at 10:13, Aymeric Augustin < aymeric.augus...@polytechnique.org> wrote:

Re: Implement QuerySet.__contains__?

2020-06-02 Thread Aymeric Augustin
Hello Johan, You explained the upside. There's one downside to be aware of. If you already fetched the queryset, `if obj in queryset` will make a new database query, which will be slower than walking through the already fetched data (unless the queryset is really large and the database really

Implement QuerySet.__contains__?

2020-06-02 Thread Johan Schiff
Is there a specific reason Djangos QuerySet does not implement __contains__? It doesn't seem very complicated to me, but maybe I'm missing something. When checking if an object is in e queryset I always use code lite this: if queryset.filter(pk=obj.pk).exists(): The pythonic way would be: if

Re: QuerySet __contains__

2009-12-08 Thread Alex Gaynor
On Tue, Dec 8, 2009 at 10:49 PM, Jeremy Dunck wrote: > On Tue, Dec 8, 2009 at 8:38 PM, Luke Plant wrote: >> On Wednesday 09 December 2009 01:52:48 Jeremy Dunck wrote: > ... >>> You could also inspect the item to see if it's an instance of the >>> .model;

Re: QuerySet __contains__

2009-12-08 Thread Jeremy Dunck
On Tue, Dec 8, 2009 at 8:38 PM, Luke Plant wrote: > On Wednesday 09 December 2009 01:52:48 Jeremy Dunck wrote: ... >> You could also inspect the item to see if it's an instance of the >> .model; if not, fast path False. >> >> Which leads to a question of edge-case semantics

Re: QuerySet __contains__

2009-12-08 Thread Luke Plant
On Wednesday 09 December 2009 01:52:48 Jeremy Dunck wrote: > On Tue, Dec 8, 2009 at 7:22 PM, Luke Plant > wrote: ... > > > However, it could be slightly more efficient in some cases, > > because the entire QuerySet._result_cache does not necessarily > > need to be filled

Re: QuerySet __contains__

2009-12-08 Thread Jeremy Dunck
On Tue, Dec 8, 2009 at 7:22 PM, Luke Plant wrote: ... > However, it could be slightly more efficient in some cases, because > the entire QuerySet._result_cache does not necessarily need to be > filled - we can stop if we find a match, saving us the work of > building Model

Re: QuerySet __contains__

2009-12-08 Thread Alex Gaynor
On Tue, Dec 8, 2009 at 8:22 PM, Luke Plant wrote: > Hi all, > > I discovered that QuerySet supports the 'in' operator: > >  myset = Articles.objects.filter(foo=bar) >  if someobject in myset: >      # etc. > > The Python docs I could find imply (but don't state) that if

QuerySet __contains__

2009-12-08 Thread Luke Plant
Hi all, I discovered that QuerySet supports the 'in' operator: myset = Articles.objects.filter(foo=bar) if someobject in myset: # etc. The Python docs I could find imply (but don't state) that if there is no __contains__() method, but there is __getitem__(), then __getitem__() is