You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@steve.apache.org by Greg Stein <gs...@gmail.com> on 2015/03/26 12:16:00 UTC
Re: svn commit: r1669304 - /steve/trunk/pysteve/lib/plugins/fic.py
On Thu, Mar 26, 2015 at 4:17 AM, <hu...@apache.org> wrote:
>...
> +++ steve/trunk/pysteve/lib/plugins/fic.py Thu Mar 26 09:17:56 2015
>
>...
> @@ -65,15 +65,17 @@ def tallyFIC(votes, issue):
> matrix[letter] += numseats - i
> i += 1
>
> +
> # Start counting
> - winners = [l for l in matrix if matrix[l] in heapq.nlargest(numseats,
> matrix.values())]
> + sorted_matrix = sorted(matrix.items(), key=operator.itemgetter(1))
> + winners = [l[0] for l in sorted_matrix if matrix[l[0]] in
> heapq.nlargest(numseats, matrix.values())]
>
> # Compile list of winner names
> winnernames = []
> x = 0
> for c in winners:
> i = ord(c) - ord('a')
> - winnernames.append("%s ( %u)" % ( candidates[i],
> heapq.nlargest(numseats, matrix.values())[x]))
> + winnernames.append("%s (%u points)" % ( candidates[i],
> heapq.nlargest(numseats, matrix.values())[x]))
>
I realize that we aren't dealing with time sensitive code, and the data
sets are not large ... but the above irks me. The heapq.nlargest() is
performed on each iteration of the list comprehension, and on each
iteration of the for-loop. Yet the result appears to be constant across all
these loops. ?
Cheers,
-g
Re: svn commit: r1669304 - /steve/trunk/pysteve/lib/plugins/fic.py
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 26, 2015 at 6:21 AM, Daniel Gruno <hu...@apache.org> wrote:
>
>
> On 2015-03-26 12:16, Greg Stein wrote:
>
>> On Thu, Mar 26, 2015 at 4:17 AM, <hu...@apache.org> wrote:
>>
>>> ...
>>> +++ steve/trunk/pysteve/lib/plugins/fic.py Thu Mar 26 09:17:56 2015
>>>
>>> ...
>>> @@ -65,15 +65,17 @@ def tallyFIC(votes, issue):
>>> matrix[letter] += numseats - i
>>> i += 1
>>>
>>> +
>>> # Start counting
>>> - winners = [l for l in matrix if matrix[l] in
>>> heapq.nlargest(numseats,
>>> matrix.values())]
>>> + sorted_matrix = sorted(matrix.items(), key=operator.itemgetter(1))
>>> + winners = [l[0] for l in sorted_matrix if matrix[l[0]] in
>>> heapq.nlargest(numseats, matrix.values())]
>>>
>>> # Compile list of winner names
>>> winnernames = []
>>> x = 0
>>> for c in winners:
>>> i = ord(c) - ord('a')
>>> - winnernames.append("%s ( %u)" % ( candidates[i],
>>> heapq.nlargest(numseats, matrix.values())[x]))
>>> + winnernames.append("%s (%u points)" % ( candidates[i],
>>> heapq.nlargest(numseats, matrix.values())[x]))
>>>
>>> I realize that we aren't dealing with time sensitive code, and the data
>> sets are not large ... but the above irks me. The heapq.nlargest() is
>> performed on each iteration of the list comprehension, and on each
>> iteration of the for-loop. Yet the result appears to be constant across
>> all
>> these loops. ?
>>
> Yeah, thanks for spotting that - t'was a silly mistake of shortening the
> code, thereby actually making more work for the interpreter.
> I've committed a change where nlargest is called just once now.
>
> Again, thanks for the review :)
No problem. But don't thank me yet... my next one isn't nice :-P
Cheers,
-g
Re: svn commit: r1669304 - /steve/trunk/pysteve/lib/plugins/fic.py
Posted by Daniel Gruno <hu...@apache.org>.
On 2015-03-26 12:16, Greg Stein wrote:
> On Thu, Mar 26, 2015 at 4:17 AM, <hu...@apache.org> wrote:
>> ...
>> +++ steve/trunk/pysteve/lib/plugins/fic.py Thu Mar 26 09:17:56 2015
>>
>> ...
>> @@ -65,15 +65,17 @@ def tallyFIC(votes, issue):
>> matrix[letter] += numseats - i
>> i += 1
>>
>> +
>> # Start counting
>> - winners = [l for l in matrix if matrix[l] in heapq.nlargest(numseats,
>> matrix.values())]
>> + sorted_matrix = sorted(matrix.items(), key=operator.itemgetter(1))
>> + winners = [l[0] for l in sorted_matrix if matrix[l[0]] in
>> heapq.nlargest(numseats, matrix.values())]
>>
>> # Compile list of winner names
>> winnernames = []
>> x = 0
>> for c in winners:
>> i = ord(c) - ord('a')
>> - winnernames.append("%s ( %u)" % ( candidates[i],
>> heapq.nlargest(numseats, matrix.values())[x]))
>> + winnernames.append("%s (%u points)" % ( candidates[i],
>> heapq.nlargest(numseats, matrix.values())[x]))
>>
> I realize that we aren't dealing with time sensitive code, and the data
> sets are not large ... but the above irks me. The heapq.nlargest() is
> performed on each iteration of the list comprehension, and on each
> iteration of the for-loop. Yet the result appears to be constant across all
> these loops. ?
Yeah, thanks for spotting that - t'was a silly mistake of shortening the
code, thereby actually making more work for the interpreter.
I've committed a change where nlargest is called just once now.
Again, thanks for the review :)
> Cheers,
> -g
>