Ticket #21751 review requested

2014-01-16 Thread Michael Manfre 
In an effort to make Django a bit more explicit with regards to closing
cursors when they are no longer needed, I have created ticket #21751 [1]
with a pull request[2].

Most of the changes are straight forward and change the usage of a cursor
so that it uses a context manager or closes the cursor in 'finally'.

Example:

# old pattern
connection.cursor().execute(...)

# changes to
with connection.cursor() as cursor:
   cursor.execute(...)

The biggest change is with SQLCompiler.execute_sql. I added new constants
to make the usage of execute_sql more explicit (and self documenting) for
the cases when the caller wants no result and when the caller wants to
fetch the results from the cusor directly.

[1] https://code.djangoproject.com/ticket/21751
[2] https://github.com/django/django/pull/2154

Regards,
Michael Manfre

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAGdCwBv1nO%3D-H9j-nHUXTsdC-V0OF50fOgbYBXhuWs%2BGeB4M0Q%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Improving aggregate support (#14030)

2014-01-16 Thread Josh Smeaton 
That's fine, I knew I was cutting it close when I started. At least I can start 
getting to bed at a reasonable time now :)

I was wondering where the dev Irc channel was, so I'll make use of that too. 
Thanks for the pointers so far. 

- Josh

> On 16 Jan 2014, at 19:36, Anssi Kääriäinen  wrote:
> 
> I'm sorry I haven't had enough time to review your work. Unfortunately I 
> won't have any more time in the near future either. So, it looks like this 
> has to wait for 1.8. Sorry for that.
> 
> If you want to ask specific questions, #django-dev IRC channel is a good 
> place to ask those questions. It is often easier to solve design issues 
> real-time than on a mailing list.
> 
>  - Anssi
> 
>> On Wednesday, January 15, 2014 3:57:33 PM UTC+2, Josh Smeaton wrote:
>> Quick update. GIS support is nearly done, I just have two failing tests to 
>> take care of (one to do with lookups setting srid, and another that is 
>> producing the wrong value). I have a few validation checks to make inside 
>> prepare, and then it's on to writing tests that make use of aggregates and 
>> expressions together. I'm unsure what kind of expressions I can write with 
>> GIS queries though, so if anyone has any suggestions, I'd be glad to 
>> implement them.
>> 
>> Examples of tests with regular aggregates would be:
>> 
>> .annotate(Sum('field')+Sum('anotherfield'))
>> .annotate(Max('field')*2)
>> 
>> I'm not sure how you'd compose GeoAggregates as I've never used them before.
>> 
>> I should mention that I've only run GIS tests against PostGIS. I've noticed 
>> that Oracle is special cased in a number of areas, so I'd appreciate any 
>> help I could receive in testing against Oracle spatial.
>> 
>> The exact commit is: 
>> https://github.com/jarshwah/django/commit/c892c0076398e6aad134fc8df549a54a94ad9dd1
>> 
>> There was a bug in gis/sql/compiler that passed the quote_name function 
>> instead of itself to the expression. Whether or not this change is accepted, 
>> that bug should be fixed in master as it breaks the query expression API.
>> 
>> Cheers,
>> 
>> - Josh
> 
> -- 
> You received this message because you are subscribed to a topic in the Google 
> Groups "Django developers" group.
> To unsubscribe from this topic, visit 
> https://groups.google.com/d/topic/django-developers/8vEAwSwJGMc/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to 
> django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/7c5a1866-a414-48c6-af4a-6477b8b5b79a%40googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/B0BAF75C-52D4-49A8-8939-70D2E96E32C2%40gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Improving aggregate support (#14030)

2014-01-16 Thread Anssi Kääriäinen 
I'm sorry I haven't had enough time to review your work. Unfortunately I 
won't have any more time in the near future either. So, it looks like this 
has to wait for 1.8. Sorry for that.

If you want to ask specific questions, #django-dev IRC channel is a good 
place to ask those questions. It is often easier to solve design issues 
real-time than on a mailing list.

 - Anssi

On Wednesday, January 15, 2014 3:57:33 PM UTC+2, Josh Smeaton wrote:
>
> Quick update. GIS support is nearly done, I just have two failing tests to 
> take care of (one to do with lookups setting srid, and another that is 
> producing the wrong value). I have a few validation checks to make inside 
> prepare, and then it's on to writing tests that make use of aggregates and 
> expressions together. I'm unsure what kind of expressions I can write with 
> GIS queries though, so if anyone has any suggestions, I'd be glad to 
> implement them.
>
> Examples of tests with regular aggregates would be:
>
> .annotate(Sum('field')+Sum('anotherfield'))
> .annotate(Max('field')*2)
>
> I'm not sure how you'd compose GeoAggregates as I've never used them 
> before.
>
> I should mention that I've only run GIS tests against PostGIS. I've 
> noticed that Oracle is special cased in a number of areas, so I'd 
> appreciate any help I could receive in testing against Oracle spatial.
>
> The exact commit is: 
> https://github.com/jarshwah/django/commit/c892c0076398e6aad134fc8df549a54a94ad9dd1
>
> There was a bug in gis/sql/compiler that passed the quote_name function 
> instead of itself to the expression. Whether or not this change is 
> accepted, that bug should be fixed in master as it breaks the query 
> expression API.
>
> Cheers,
>
> - Josh
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/7c5a1866-a414-48c6-af4a-6477b8b5b79a%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.