← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:py3-cmp into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:py3-cmp into launchpad:master.

Commit message:
Stop using cmp() and sorted(cmp=...)

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/394483

These are no longer supported in Python 3; we need to use constructions involving sorted(key=...) instead.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-cmp into launchpad:master.
diff --git a/lib/lp/archivepublisher/domination.py b/lib/lp/archivepublisher/domination.py
index 356a2ee..0e9a8b0 100644
--- a/lib/lp/archivepublisher/domination.py
+++ b/lib/lp/archivepublisher/domination.py
@@ -54,6 +54,7 @@ __all__ = ['Dominator']
 
 from collections import defaultdict
 from datetime import timedelta
+from functools import cmp_to_key
 from operator import (
     attrgetter,
     itemgetter,
@@ -198,14 +199,16 @@ class GeneralizedPublication:
             self.getPackageVersion(pub1), self.getPackageVersion(pub2))
 
         if version_comparison == 0:
-            # Use dates as tie breaker.
-            return cmp(pub1.datecreated, pub2.datecreated)
+            # Use dates as tie breaker (idiom equivalent to Python 2's cmp).
+            return (
+                (pub1.datecreated > pub2.datecreated) -
+                (pub1.datecreated < pub2.datecreated))
         else:
             return version_comparison
 
     def sortPublications(self, publications):
         """Sort publications from most to least current versions."""
-        return sorted(publications, cmp=self.compare, reverse=True)
+        return sorted(publications, key=cmp_to_key(self.compare), reverse=True)
 
 
 def find_live_source_versions(sorted_pubs):
@@ -417,7 +420,8 @@ class Dominator:
             len(sorted_pubs), live_versions)
 
         # Verify that the publications are really sorted properly.
-        check_order = OrderingCheck(cmp=generalization.compare, reverse=True)
+        check_order = OrderingCheck(
+            key=cmp_to_key(generalization.compare), reverse=True)
 
         current_dominant = None
         dominant_version = None
diff --git a/lib/lp/archivepublisher/tests/test_dominator.py b/lib/lp/archivepublisher/tests/test_dominator.py
index 4d073e0..b35596e 100755
--- a/lib/lp/archivepublisher/tests/test_dominator.py
+++ b/lib/lp/archivepublisher/tests/test_dominator.py
@@ -8,6 +8,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 __metaclass__ = type
 
 import datetime
+from functools import cmp_to_key
 from operator import attrgetter
 
 import apt_pkg
@@ -559,7 +560,8 @@ class TestGeneralizedPublication(TestCaseWithFactory):
             '1.1v3',
             ]
         spphs = make_spphs_for_versions(self.factory, versions)
-        sorted_spphs = sorted(spphs, cmp=GeneralizedPublication().compare)
+        sorted_spphs = sorted(
+            spphs, key=cmp_to_key(GeneralizedPublication().compare))
         self.assertEqual(
             sorted(versions), list_source_versions(sorted_spphs))
 
@@ -572,16 +574,18 @@ class TestGeneralizedPublication(TestCaseWithFactory):
             ]
         spphs = make_spphs_for_versions(self.factory, versions)
 
-        debian_sorted_versions = sorted(versions, cmp=apt_pkg.version_compare)
+        debian_sorted_versions = sorted(
+            versions, key=cmp_to_key(apt_pkg.version_compare))
 
         # Assumption: in this case, Debian version ordering is not the
         # same as alphabetical version ordering.
         self.assertNotEqual(sorted(versions), debian_sorted_versions)
 
         # The compare method produces the Debian ordering.
-        sorted_spphs = sorted(spphs, cmp=GeneralizedPublication().compare)
+        sorted_spphs = sorted(
+            spphs, key=cmp_to_key(GeneralizedPublication().compare))
         self.assertEqual(
-            sorted(versions, cmp=apt_pkg.version_compare),
+            sorted(versions, key=cmp_to_key(apt_pkg.version_compare)),
             list_source_versions(sorted_spphs))
 
     def test_compare_breaks_tie_with_creation_date(self):
@@ -605,7 +609,7 @@ class TestGeneralizedPublication(TestCaseWithFactory):
 
         self.assertEqual(
             [spphs[2], spphs[0], spphs[1]],
-            sorted(spphs, cmp=GeneralizedPublication().compare))
+            sorted(spphs, key=cmp_to_key(GeneralizedPublication().compare)))
 
     def test_compare_breaks_tie_for_releases_with_same_version(self):
         # When two publications are tied for comparison because they
@@ -629,7 +633,7 @@ class TestGeneralizedPublication(TestCaseWithFactory):
 
         self.assertEqual(
             [spphs[2], spphs[0], spphs[1]],
-            sorted(spphs, cmp=GeneralizedPublication().compare))
+            sorted(spphs, key=cmp_to_key(GeneralizedPublication().compare)))
 
 
 def jumble(ordered_list):
diff --git a/lib/lp/archiveuploader/tests/nascentupload-announcements.txt b/lib/lp/archiveuploader/tests/nascentupload-announcements.txt
index 017601f..cd512c3 100644
--- a/lib/lp/archiveuploader/tests/nascentupload-announcements.txt
+++ b/lib/lp/archiveuploader/tests/nascentupload-announcements.txt
@@ -38,14 +38,11 @@ We need to be logged into the security model in order to get any further
     >>> switch_dbuser('launchpad')
     >>> login('foo.bar@xxxxxxxxxxxxx')
 
-Helper functions to examine emails that were sent:
+A helper function to examine emails that were sent:
 
     >>> from lp.services.mail import stub
     >>> from lp.testing.mail_helpers import pop_notifications
 
-    >>> def by_to_addrs(a, b):
-    ...     return cmp(a[1], b[1])
-
     >>> def print_addrlist(field):
     ...    for entry in sorted([addr.strip() for addr in field.split(',')]):
     ...        print entry
diff --git a/lib/lp/bugs/doc/initial-bug-contacts.txt b/lib/lp/bugs/doc/initial-bug-contacts.txt
index 468658c..cd88e6f 100644
--- a/lib/lp/bugs/doc/initial-bug-contacts.txt
+++ b/lib/lp/bugs/doc/initial-bug-contacts.txt
@@ -157,12 +157,10 @@ track why daf received the email. The rational is repeated in the footer
 of the email with the bug title and URL.
 
     >>> import email
-
-    >>> def by_to_addrs(a, b):
-    ...        return cmp(a[1], b[1])
+    >>> from operator import itemgetter
 
     >>> test_emails = list(stub.test_emails)
-    >>> test_emails.sort(by_to_addrs)
+    >>> test_emails.sort(key=itemgetter(1))
 
     >>> len(test_emails)
     1
diff --git a/lib/lp/registry/browser/product.py b/lib/lp/registry/browser/product.py
index e7dbe7e..36109fe 100644
--- a/lib/lp/registry/browser/product.py
+++ b/lib/lp/registry/browser/product.py
@@ -664,14 +664,12 @@ class ProductSpecificationsMenu(NavigationMenu, ProductEditLinksMixin,
              'register_sprint']
 
 
-def _cmp_distros(a, b):
+def _distro_name_sort_key(name):
     """Put Ubuntu first, otherwise in alpha order."""
-    if a == 'ubuntu':
-        return -1
-    elif b == 'ubuntu':
-        return 1
+    if name == 'ubuntu':
+        return (0, )
     else:
-        return cmp(a, b)
+        return (1, name)
 
 
 class ProductSetBreadcrumb(Breadcrumb):
@@ -1227,8 +1225,7 @@ class ProductPackagesView(LaunchpadView):
                 distros[distribution.name] = distro
             distro['packagings'].append(packaging)
         # Now we sort the resulting list of "distro" objects, and return that.
-        distro_names = distros.keys()
-        distro_names.sort(cmp=_cmp_distros)
+        distro_names = sorted(distros, key=_distro_name_sort_key)
         results = [distros[name] for name in distro_names]
         return results
 
diff --git a/lib/lp/registry/doc/distribution-mirror.txt b/lib/lp/registry/doc/distribution-mirror.txt
index 90bc89e..43e129c 100644
--- a/lib/lp/registry/doc/distribution-mirror.txt
+++ b/lib/lp/registry/doc/distribution-mirror.txt
@@ -341,10 +341,11 @@ up on the public mirror listings.
     >>> transaction.commit()
 
     >>> import email
+    >>> from operator import itemgetter
     >>> from lp.services.mail import stub
     >>> len(stub.test_emails)
     2
-    >>> stub.test_emails.sort(lambda a, b: cmp(a[1], b[1])) # sort by to_addr
+    >>> stub.test_emails.sort(key=itemgetter(1))  # sort by to_addr
     >>> from_addr, to_addr, raw_message = stub.test_emails.pop()
     >>> msg = email.message_from_string(raw_message)
     >>> msg['To']
diff --git a/lib/lp/registry/doc/teammembership-email-notification.txt b/lib/lp/registry/doc/teammembership-email-notification.txt
index c1c323f..201cc48 100644
--- a/lib/lp/registry/doc/teammembership-email-notification.txt
+++ b/lib/lp/registry/doc/teammembership-email-notification.txt
@@ -7,8 +7,7 @@ where we might want to notify only the team admins, but in most of the
 cases we'll be sending two similar (but not identical) notifications:
 one for all team admins and another for the member.
 
-    >>> def by_to_addrs(a, b):
-    ...     return cmp(a[1], b[1])
+    >>> from operator import itemgetter
 
     >>> def setStatus(membership, status, reviewer=None, comment=None,
     ...               silent=False):
@@ -186,7 +185,7 @@ The same goes for approving a proposed member.
     >>> setStatus(daf_membership, TeamMembershipStatus.APPROVED,
     ...           reviewer=mark, comment='This is a nice guy; I like him')
     >>> run_mail_jobs()
-    >>> stub.test_emails.sort(by_to_addrs)
+    >>> stub.test_emails.sort(key=itemgetter(1))
     >>> len(stub.test_emails)
     6
 
@@ -236,7 +235,7 @@ The same for deactivating a membership.
     >>> setStatus(daf_membership, TeamMembershipStatus.DEACTIVATED,
     ...           reviewer=mark)
     >>> run_mail_jobs()
-    >>> stub.test_emails.sort(by_to_addrs)
+    >>> stub.test_emails.sort(key=itemgetter(1))
     >>> len(stub.test_emails)
     6
 
diff --git a/lib/lp/registry/tests/test_distroseriesdifferencecomment.py b/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
index beeaecd..f95aac7 100644
--- a/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
+++ b/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
@@ -6,7 +6,7 @@
 __metaclass__ = type
 
 from datetime import timedelta
-from random import randint
+from random import random
 
 from storm.store import Store
 from zope.component import getUtility
@@ -31,14 +31,9 @@ def get_comment_source():
     return getUtility(IDistroSeriesDifferenceCommentSource)
 
 
-def flip_coin(*args):
-    """Random comparison function.  Returns -1 or 1 randomly."""
-    return 1 - 2 * randint(0, 1)
-
-
 def randomize_list(original_list):
     """Sort a list (or other iterable) in random order.  Return list."""
-    return sorted(original_list, cmp=flip_coin)
+    return sorted(original_list, key=lambda _: random)
 
 
 class DistroSeriesDifferenceCommentTestCase(TestCaseWithFactory):
diff --git a/lib/lp/services/doc/orderingcheck.txt b/lib/lp/services/doc/orderingcheck.txt
index 9ed2b0b..fe9294f 100644
--- a/lib/lp/services/doc/orderingcheck.txt
+++ b/lib/lp/services/doc/orderingcheck.txt
@@ -29,10 +29,7 @@ Sorting criteria
 The OrderingCheck accepts all the same sorting options (as keyword args)
 as Python's built-in sorting functions.
 
-    >>> def sort_cmp(left_item, right_item):
-    ...     return left_item - right_item
-
-    >>> checker = OrderingCheck(cmp=sort_cmp, reverse=True)
+    >>> checker = OrderingCheck(key=lambda v: v, reverse=True)
     >>> for number in (3, 2, 1):
     ...     checker.check(number)
 
diff --git a/lib/lp/services/webapp/sorting.py b/lib/lp/services/webapp/sorting.py
index dc1a453..6d34142 100644
--- a/lib/lp/services/webapp/sorting.py
+++ b/lib/lp/services/webapp/sorting.py
@@ -16,14 +16,14 @@ import six
 def expand_numbers(unicode_text, fill_digits=4):
     """Return a copy of the string with numbers zero filled.
 
-    >>> expand_numbers(u'hello world')
-    u'hello world'
-    >>> expand_numbers(u'0.12.1')
-    u'0000.0012.0001'
-    >>> expand_numbers(u'0.12.1', 2)
-    u'00.12.01'
-    >>> expand_numbers(u'branch-2-3.12')
-    u'branch-0002-0003.0012'
+    >>> print(expand_numbers(u'hello world'))
+    hello world
+    >>> print(expand_numbers(u'0.12.1'))
+    0000.0012.0001
+    >>> print(expand_numbers(u'0.12.1', 2))
+    00.12.01
+    >>> print(expand_numbers(u'branch-2-3.12'))
+    branch-0002-0003.0012
 
     """
     assert(isinstance(unicode_text, six.text_type))
@@ -40,26 +40,22 @@ reversed_numbers_table = dict(
   zip(map(ord, u'0123456789'), reversed(u'0123456789')))
 
 
-def _reversed_number_comparator(lhs_text, rhs_text):
+def _reversed_number_sort_key(text):
     """Return comparison value reversed for numbers only.
 
-    >>> _reversed_number_comparator(u'9.3', u'2.4')
-    -1
-    >>> _reversed_number_comparator(u'world', u'hello')
-    1
-    >>> _reversed_number_comparator(u'hello world', u'hello world')
-    0
-    >>> _reversed_number_comparator(u'dev', u'development')
-    -1
-    >>> _reversed_number_comparator(u'bzr-0.13', u'bzr-0.08')
-    -1
+    >>> print(_reversed_number_sort_key(u'9.3'))
+    0.6
+    >>> print(_reversed_number_sort_key(u'2.4'))
+    7.5
+    >>> print(_reversed_number_sort_key(u'hello'))
+    hello
+    >>> print(_reversed_number_sort_key(u'bzr-0.13'))
+    bzr-9.86
 
     """
-    assert isinstance(lhs_text, six.text_type)
-    assert isinstance(rhs_text, six.text_type)
-    translated_lhs_text = lhs_text.translate(reversed_numbers_table)
-    translated_rhs_text = rhs_text.translate(reversed_numbers_table)
-    return cmp(translated_lhs_text, translated_rhs_text)
+    assert isinstance(text, six.text_type)
+    assert isinstance(text, six.text_type)
+    return text.translate(reversed_numbers_table)
 
 
 def _identity(x):
@@ -71,13 +67,13 @@ def sorted_version_numbers(sequence, key=_identity):
 
     >>> bzr_versions = [u'0.9', u'0.10', u'0.11']
     >>> for version in sorted_version_numbers(bzr_versions):
-    ...   print version
+    ...   print(version)
     0.11
     0.10
     0.9
     >>> bzr_versions = [u'bzr-0.9', u'bzr-0.10', u'bzr-0.11']
     >>> for version in sorted_version_numbers(bzr_versions):
-    ...   print version
+    ...   print(version)
     bzr-0.11
     bzr-0.10
     bzr-0.9
@@ -91,7 +87,7 @@ def sorted_version_numbers(sequence, key=_identity):
     >>> from operator import attrgetter
     >>> for version in sorted_version_numbers(bzr_versions,
     ...                                       key=attrgetter('name')):
-    ...   print version.name
+    ...   print(version.name)
     0.11
     0.10
     0.9
@@ -101,9 +97,9 @@ def sorted_version_numbers(sequence, key=_identity):
     foo
 
     """
-    expanded_key = lambda x: expand_numbers(key(x))
-    return sorted(sequence, key=expanded_key,
-                  cmp=_reversed_number_comparator)
+    return sorted(
+        sequence,
+        key=lambda x: _reversed_number_sort_key(expand_numbers(key(x))))
 
 
 def sorted_dotted_numbers(sequence, key=_identity):
@@ -117,13 +113,13 @@ def sorted_dotted_numbers(sequence, key=_identity):
 
     >>> bzr_versions = [u'0.9', u'0.10', u'0.11']
     >>> for version in sorted_dotted_numbers(bzr_versions):
-    ...   print version
+    ...   print(version)
     0.9
     0.10
     0.11
     >>> bzr_versions = [u'bzr-0.9', u'bzr-0.10', u'bzr-0.11']
     >>> for version in sorted_dotted_numbers(bzr_versions):
-    ...   print version
+    ...   print(version)
     bzr-0.9
     bzr-0.10
     bzr-0.11
@@ -137,7 +133,7 @@ def sorted_dotted_numbers(sequence, key=_identity):
     >>> from operator import attrgetter
     >>> for version in sorted_dotted_numbers(bzr_versions,
     ...                                      key=attrgetter('name')):
-    ...   print version.name
+    ...   print(version.name)
     0.9
     0.10
     0.11
@@ -147,5 +143,4 @@ def sorted_dotted_numbers(sequence, key=_identity):
     foo
 
     """
-    expanded_key = lambda x: expand_numbers(key(x))
-    return sorted(sequence, key=expanded_key)
+    return sorted(sequence, key=lambda x: expand_numbers(key(x)))