gistfile1.txt

04:52 cramm: so, I think the patch that gives .values() the ability to specify aliases, e.g. values(foo='field__at_the_end_of_a_chain__of__related_models') is in good shape
04:56 cramm: what I'm still fighting against is to get chaining of values(alias=...).aggregate(expression involving alias) and  values(alias=...).annotate(expression involving alias) to work
04:58 cramm: but a good group of other QS chaining constructs do work: https://github.com/django/django/pull/2510/files#diff-db5fb8fdfbba5c6ac7595dd636d9b558R254
04:58 edbaffei has left IRC (Quit: edbaffei)
04:59 cramm: do you gouys whing it would be worthwhile to commit^Wask for review this in two parts? the one that is working now and the other once I can tame it and/or get some help?
05:01 jkocherhans has left IRC (Quit: Textual IRC Client: www.textualapp.com)
05:07 dlogs has joined ([email protected])
05:09 ojii has left IRC (Ping timeout: 256 seconds)
05:20 tonebot has left IRC (Remote host closed the connection)
05:20 tonebot has joined ([email protected])
06:03 PirosB3 has joined ([email protected])
06:06 dlogs has left IRC (Remote host closed the connection)
06:07 dlogs has joined ([email protected])
06:42 ustunozgur has left IRC (Remote host closed the connection)
07:07 dlogs has left IRC (Remote host closed the connection)
07:07 dlogs has joined ([email protected])
07:19 PirosB3 has left IRC (Quit: PirosB3)
07:43 trbs2 has left IRC (Quit: Leaving)
07:43 timograham has left ()
07:46 stephenmcd has joined (~steve@mezzanine/stephenmcd)
08:00 jarshwah: cramm: are you still around?
08:04 cramm: jarshwah: I am
08:05 jarshwah: re: getting .aggregate() to work with value aliases.. I think my patch that handles non-aggregate annotations should probably work with it
08:05 jarshwah: (I haven't tested it though)
08:05 jarshwah: but even if it doesn't, it shouldn't be a lot of work to get it going
08:05 jarshwah: if the patch above makes it to master, I'll rebase and get it working
08:05 jarshwah: https://github.com/django/django/pull/2496 for reference
08:06 jarshwah: (that patch you links to scares me.. going to make my patch harder to merge, guaranteed)
08:06 dlogs has left IRC (Remote host closed the connection)
08:06 cramm: jarshwah: do yu mean https://github.com/django/django/pull/2184 ?
08:07 dlogs has joined ([email protected])
08:07 cramm: jarshwah: ah yes, you opened a more recent PR
08:07 cramm: jarshwah: I know one of the PR's s going to break things for the other. I will work so we gen both of them merged into master
08:08 cramm: jarshwah: Thanks for a great work there, btw
08:08 jarshwah: I'm just looking over the values alias one now to see if I can find anything that jumps out at me
08:08 jarshwah: yeah no worries, it was fun
08:09 cramm: I'm running tests and cleaning up the branch from which the PR is opened
08:09 jarshwah: as soon as this is merged, I've got some other things I'd like to work on
08:09 cramm: I will leave there only the working bits I mentioned above
08:09 jarshwah: you mean 2510?
08:09 cramm: yes
08:10 cramm: My plan is to leave out for now the three last commits, and propose we merge until e87c18b
08:10 ticketbot: https://github.com/django/django/commit/e87c18b
08:11 jarshwah: are you @ramiro ?
08:11 cramm: jarshwah: yes
08:11 jarshwah: right that makes sense now
08:11 jarshwah: :P
08:12 cramm: btw I'm simply trying to update the great patch from two years ago by user nate_b in ticket #16735
08:12 ticketbot: https://code.djangoproject.com/ticket/16735
08:12 jarshwah: did nate_b disappear? he wrote lots of good patches
08:12 jarshwah: that's where my patch originated from too (in a way)
08:12 jarshwah: I started from scratch, but took a lot of his learnings away
08:13 cramm: I think so, at least I can't map his nickname to anyone I know
08:15 cramm: jarshwah: I hven't loooked at your latest PR so probably you've already taken care of this, but don't forget about python 3 and geodjango :)
08:15 jarshwah: took care of both =)
08:15 cramm: I needed to add compatibility for them to bendavis78's patch
08:15 cramm: great
08:16 jarshwah: gis was made to work, not implement the new features though
08:16 jarshwah: as in.. theres no GeoAggregate()+GeoAggregate() support like there is Sum()+Sum()
08:18 jarshwah: anyways I'll take a look at this patch and see if I can offer any criticism
08:19 cramm: jarshwah: thanks
08:25 jarshwah: ok the first thing I've got to mention is..
08:25 jarshwah: instead of having a _fields and an _aliased_fields spread all over the place
08:25 jarshwah: convert positional fields into relevant kw aliased fields
08:26 jarshwah: I'm not sure how much you've put into this patch, but I found supporting positional and kwargs differently causes a whole lot of pain
08:49 jarshwah: hmm that's the other thing.. my patch allows: Model.objects.annotate(alias=F('some_long_name')).values('alias')
08:49 jarshwah: which is longer and less obvious, but achieves the same result
08:51 jarshwah: ill put this stuff on the PR anyway :P
08:57 loic84: I like Model.objects.annotate(alias=F('some_long_name')).values('alias').
08:57 loic84: It's long but most non trivial queries are anyway.
08:59 loic84: What it does is obvious at first glance.
09:00 jarshwah: what I'm attempting now is to implement .values(alias=col) internally as .annotate().values()
09:00 jarshwah: should be rather straight forward
09:00 jarshwah: then removes the "taint" from other places
09:00 cramm: jarshwah: sorry, hadn't beel looking at the IRc client
09:00 jarshwah: nah don't worry, I ramble
09:01 cramm: jarshwah: I've pushed --force to the branch, it now just contains the code that 'works' (i.e. the tests pass)
09:02 loic84: .values(alias=col, alias2=alias) does it work? :P
09:02 jarshwah: ok awesome, I'll incorporate all the tests and see if this re-implementation works fairly trivially
09:02 cramm: jarshwah: the additionnal commits are now in https://github.com/ramiro/django/compare/16735-values-alias-part2?expand=1
09:03 jarshwah: loic84: good question.. should it?
09:03 cramm: (the ones which are exploratory and contain a couple of tests that show what's failing)
09:03 loic84: why not? once it's aliased it should work.
09:04 loic84: the only probolem is that kwargs aren't ordered, got that problem with prefetch_related() when we  tried to use kwargs rather than Prefetch objects.
09:04 jarshwah: it's order dependent though
09:05 jarshwah: perhaps .values(alias=col).values(alias2=alias)
09:05 loic84: otherwise, Alias objects :P
09:05 jarshwah: although I'm not sure what happens with alias().alias()
09:05 jarshwah: errr .values().values()
09:06 jarshwah: I honestly wouldnt mind alias objects :P they make working with arguments much easier
09:06 jarshwah: pre and post processing
09:06 dlogs has left IRC (Remote host closed the connection)
09:07 dlogs has joined ([email protected])
09:08 loic84: cc akaariai_
09:09 jarshwah: also, values() could just be (internally) (alias, F('name'))
09:10 jarshwah: and that's what I was going to do after the nonaggregate lands
09:10 jarshwah: same as with the order_by() .. ('+', F('name'))
09:10 loic84: would that help getting rid of ValuesQuerySet?
09:10 jarshwah: then you get lookups/transforms for free
09:11 jarshwah: not sure, I haven't looked into it that closely
09:12 jarshwah: will be my next django project though if I beat akaariai to it