Merge lp:~sinzui/charmworld/modern-py into lp:~juju-jitsu/charmworld/trunk

[Curtis Hovey]

Modernise chartmers() and document what it really does.

Modernise chartmers() and document what it really does.

While adding API3, I stumbled reading the charmer() method. The code is written like py2.3. I uses the deprecated cmp() builtin, and it does work that defaultdict and soted do. I did not understand why the first element was popped. I then read the comment and knew it was a lie. charmworld does not know all the committers to charms. The listing is really about how many charms people own, except for the charmers team which is assumed to own most charms.

[Aaron Bentley (abentley)]

This is shorter and clearer, so I like it. I don't know if there's value in py3 compatibility per se. If it were somewhere other than report code, I'd ask about tests.

review: Approve

=== modified file 'charmworld/views/misc.py'
--- charmworld/views/misc.py	2013-08-15 20:28:10 +0000
+++ charmworld/views/misc.py	2013-08-16 13:42:08 +0000
@@ -1,6 +1,8 @@
 # Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from collections import defaultdict
+
 from pyramid.view import view_config
 
 from charmworld import cached_view_config
@@ -24,22 +26,17 @@
     route_name="charmers",
     renderer="charmworld:templates/charmers.pt")
 def charmers(request):
-    # We want to collect which users have worked on which charms.
-    # By worked on means they have at least one commit to it.
-    # collect a list of users grouped by charm name.
-    # The list of users does not correspond to the lp ids.
+    # We want to collect which users have contributes on which charms.
+    # This however reports charm owners ordered decending by ownership.
+    # The largest owner is not included because we think this is ~charmers.
     results = request.db.charms.find(
         {}, {"name": 1, 'owner': 1, 'short_url': 1})
 
-    owner_map = {}
+    owner_map = defaultdict(int)
     for i in results:
-        if not i['owner'] in owner_map:
-            owner_map[i['owner']] = 1
-        else:
-            owner_map[i['owner']] += 1
-
-    owners = owner_map.items()
-    owners.sort(lambda x, y: cmp(y[1], x[1]))
+        owner_map[i['owner']] += 1
+    owners = sorted(
+        owner_map.items(), key=lambda i: (i[1], i[0]), reverse=True)
     owners.pop(0)
     return {"owners": owners}