launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26462
[Merge] ~cjwatson/launchpad:py3-orderable into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:py3-orderable into launchpad:master.
Commit message:
Fix sorting of unorderable objects on Python 3
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/398876
On Python 3, objects without a meaningful natural ordering can't be ordered. Fix a number of places that were trying to do this: sometimes this involves using better sort keys, while sometimes we can use various strategies to avoid ordering them in the first place.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-orderable into launchpad:master.
diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py
index a9d9953..2a139c0 100644
--- a/lib/lp/bugs/browser/bugtask.py
+++ b/lib/lp/bugs/browser/bugtask.py
@@ -690,10 +690,14 @@ class BugTaskView(LaunchpadView, BugViewMixin, FeedsMixin):
"""
event_groups = self._event_groups
+ def activity_sort_key(activity):
+ target = activity.target
+ return (
+ activity.datechanged, 0 if target is None else 1, target,
+ activity.attribute)
+
def group_activities_by_target(activities):
- activities = sorted(
- activities, key=attrgetter(
- "datechanged", "target", "attribute"))
+ activities = sorted(activities, key=activity_sort_key)
return [
{"target": target, "activity": list(activity)}
for target, activity in groupby(
diff --git a/lib/lp/bugs/browser/tests/person-bug-views.txt b/lib/lp/bugs/browser/tests/person-bug-views.txt
index 439512d..5f1d5ab 100644
--- a/lib/lp/bugs/browser/tests/person-bug-views.txt
+++ b/lib/lp/bugs/browser/tests/person-bug-views.txt
@@ -356,7 +356,7 @@ particular bug (see bug 1357):
>>> commented_bugtasks_view = create_view(no_priv, '+commentedbugs')
>>> commented_bugs = list(commented_bugtasks_view.search().batch)
- >>> [bugtask.bug.id for bugtask in sorted(commented_bugs)]
+ >>> [bugtask.bug.id for bugtask in commented_bugs]
[1, 1, 1]
diff --git a/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt b/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt
index a01dbdc..bcaf316 100644
--- a/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt
+++ b/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt
@@ -215,9 +215,11 @@ been checked.
>>> transaction.commit()
+ >>> from operator import attrgetter
>>> with sourceforge.responses(trace_calls=True):
... bug_watch_updater.updateBugWatches(
- ... sourceforge, sorted(example_bug_tracker.watches))
+ ... sourceforge,
+ ... sorted(example_bug_tracker.watches, key=attrgetter('id')))
INFO Updating 1 watches for 1 bugs on http://bugs.some.where
GET http://bugs.some.where/support/tracker.php?aid=1722251
diff --git a/lib/lp/bugs/model/tests/test_bugtask.py b/lib/lp/bugs/model/tests/test_bugtask.py
index 527120e..21a91e4 100644
--- a/lib/lp/bugs/model/tests/test_bugtask.py
+++ b/lib/lp/bugs/model/tests/test_bugtask.py
@@ -11,7 +11,11 @@ import unittest
from lazr.lifecycle.snapshot import Snapshot
from storm.store import Store
-from testtools.matchers import Equals
+from testtools.matchers import (
+ Equals,
+ MatchesSetwise,
+ MatchesStructure,
+ )
from testtools.testcase import ExpectedException
import transaction
from zope.component import getUtility
@@ -199,12 +203,11 @@ class TestBugTaskCreation(TestCaseWithFactory):
bug_many, mark,
[evolution, a_distro, warty],
status=BugTaskStatus.FIXRELEASED)
- tasks = [(t.product, t.distribution, t.distroseries) for t in taskset]
- tasks.sort()
- self.assertEqual(tasks[0][2], warty)
- self.assertEqual(tasks[1][1], a_distro)
- self.assertEqual(tasks[2][0], evolution)
+ self.assertThat(taskset, MatchesSetwise(
+ MatchesStructure.byEquality(product=evolution),
+ MatchesStructure.byEquality(distribution=a_distro),
+ MatchesStructure.byEquality(distroseries=warty)))
def test_accesspolicyartifacts_updated(self):
# createManyTasks updates the AccessPolicyArtifacts related
diff --git a/lib/lp/bugs/stories/webservice/xx-bug.txt b/lib/lp/bugs/stories/webservice/xx-bug.txt
index 04480a5..6121737 100644
--- a/lib/lp/bugs/stories/webservice/xx-bug.txt
+++ b/lib/lp/bugs/stories/webservice/xx-bug.txt
@@ -312,9 +312,11 @@ Bug tasks
Each bug may be associated with one or more bug tasks. Much of the
data in a bug task is derived from the bug.
+ >>> from operator import itemgetter
>>> bug_one_bugtasks_url = bug_one['bug_tasks_collection_link']
>>> bug_one_bugtasks = sorted(webservice.get(
- ... bug_one_bugtasks_url).jsonBody()['entries'])
+ ... bug_one_bugtasks_url).jsonBody()['entries'],
+ ... key=itemgetter('self_link'))
>>> len(bug_one_bugtasks)
3
@@ -825,7 +827,6 @@ Bug subscriptions
We can get the collection of subscriptions to a bug.
- >>> from operator import itemgetter
>>> bug_one_subscriptions_url = bug_one['subscriptions_collection_link']
>>> subscriptions = webservice.get(bug_one_subscriptions_url).jsonBody()
>>> subscription_entries = sorted(
@@ -1056,7 +1057,8 @@ case aspects of the bugtask (like status) are slaved to the remote bug
report described by the bugwatch.
>>> bug_one_bug_watches = sorted(webservice.get(
- ... bug_one['bug_watches_collection_link']).jsonBody()['entries'])
+ ... bug_one['bug_watches_collection_link']).jsonBody()['entries'],
+ ... key=itemgetter('self_link'))
>>> len(bug_one_bug_watches)
4
diff --git a/lib/lp/bugs/subscribers/bug.py b/lib/lp/bugs/subscribers/bug.py
index 11851eb..d88d27b 100644
--- a/lib/lp/bugs/subscribers/bug.py
+++ b/lib/lp/bugs/subscribers/bug.py
@@ -15,6 +15,7 @@ __all__ = [
import datetime
+from operator import attrgetter
from zope.security.proxy import removeSecurityProxy
@@ -227,7 +228,7 @@ def send_bug_details_to_new_bug_subscribers(
recipients = bug.getBugNotificationRecipients()
bug_notification_builder = BugNotificationBuilder(bug, event_creator)
- for to_person in sorted(to_persons):
+ for to_person in sorted(to_persons, key=attrgetter('id')):
reason, rationale = recipients.getReason(
str(removeSecurityProxy(to_person).preferredemail.email))
subject, contents = generate_bug_add_email(
diff --git a/lib/lp/code/doc/revision.txt b/lib/lp/code/doc/revision.txt
index 46f0219..e514d69 100644
--- a/lib/lp/code/doc/revision.txt
+++ b/lib/lp/code/doc/revision.txt
@@ -112,7 +112,9 @@ sequence attribute is None.
>>> ancestry = IStore(BranchRevision).find(
... BranchRevision, BranchRevision.branch == branch)
>>> for branch_revision in sorted(ancestry,
- ... key=lambda r:(r.sequence, r.revision.id), reverse=True):
+ ... key=lambda r: (
+ ... 0 if r.sequence is None else 1, r.sequence, r.revision.id),
+ ... reverse=True):
... print(branch_revision.sequence, branch_revision.revision.id)
6 9
5 8
diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py
index 061fb1e..55a9197 100644
--- a/lib/lp/code/model/branchmergeproposal.py
+++ b/lib/lp/code/model/branchmergeproposal.py
@@ -1290,9 +1290,10 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
def getRevisionsSinceReviewStart(self):
"""Get the grouped revisions since the review started."""
+ all_comments = list(self.all_comments)
entries = [
- ((comment.date_created, -1), comment) for comment
- in self.all_comments]
+ ((comment.date_created, i - len(all_comments)), comment)
+ for i, comment in enumerate(all_comments)]
entries.extend(self._getNewerRevisions())
entries.sort()
current_group = []
diff --git a/lib/lp/registry/stories/webservice/xx-person.txt b/lib/lp/registry/stories/webservice/xx-person.txt
index 73450d0..619784c 100644
--- a/lib/lp/registry/stories/webservice/xx-person.txt
+++ b/lib/lp/registry/stories/webservice/xx-person.txt
@@ -303,8 +303,10 @@ Team memberships are first-class objects with their own URLs.
Team memberships also have data fields.
- >>> salgado_landscape = sorted(webservice.get(
- ... salgado_memberships).jsonBody()['entries'])[1]
+ >>> salgado_landscape = [
+ ... entry for entry in webservice.get(
+ ... salgado_memberships).jsonBody()['entries']
+ ... if entry['team_link'].endswith('~landscape-developers')][0]
>>> for key in sorted(salgado_landscape):
... print(key)
date_expires
diff --git a/lib/lp/registry/tests/test_distroseriesdifferencecomment.py b/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
index f95aac7..6276647 100644
--- a/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
+++ b/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
@@ -33,7 +33,7 @@ def get_comment_source():
def randomize_list(original_list):
"""Sort a list (or other iterable) in random order. Return list."""
- return sorted(original_list, key=lambda _: random)
+ 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 209b6e0..e2367aa 100644
--- a/lib/lp/services/doc/orderingcheck.txt
+++ b/lib/lp/services/doc/orderingcheck.txt
@@ -10,8 +10,12 @@ seem worth the trouble.
>>> from lp.services.orderingcheck import OrderingCheck
>>> def sort_key(item):
- ... """Simple sorting key for integers: the integer itself."""
- ... return item
+ ... """Simple sorting key for integers.
+ ...
+ ... This could just be the integer itself, but in order to support
+ ... None on Python 3 we need an additional term.
+ ... """
+ ... return (0 if item is None else 1, item)
The OrderingCheck makes it clean and easy. You create an OrderingCheck
with the same arguments that go into Python's standard sorting
@@ -51,7 +55,7 @@ error.
Edge cases
----------
-It is safe to use the None value. Python places it below any other
+It is safe to use the None value. sort_key places it below any other
integer.
>>> checker = OrderingCheck(key=sort_key)
diff --git a/lib/lp/services/webservice/stories/multiversion.txt b/lib/lp/services/webservice/stories/multiversion.txt
index 22f9722..683325f 100644
--- a/lib/lp/services/webservice/stories/multiversion.txt
+++ b/lib/lp/services/webservice/stories/multiversion.txt
@@ -46,11 +46,14 @@ In the 'beta' version of the web service, mutator methods like
IBugTask.transitionToStatus are published as named operations. In
subsequent versions, those named operations are not published.
+ >>> from operator import itemgetter
+
>>> def get_bugtask_path(version):
... bug_one = webservice.get("/bugs/1", api_version=version).jsonBody()
... bug_one_bugtasks_url = bug_one['bug_tasks_collection_link']
... bug_one_bugtasks = sorted(webservice.get(
- ... bug_one_bugtasks_url).jsonBody()['entries'])
+ ... bug_one_bugtasks_url).jsonBody()['entries'],
+ ... key=itemgetter('self_link'))
... bugtask_path = bug_one_bugtasks[0]['self_link']
... return bugtask_path
diff --git a/lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py b/lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py
index 0b82990..187323c 100644
--- a/lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py
+++ b/lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py
@@ -152,4 +152,6 @@ class TestScriptRunning(TestCaseWithFactory):
sorted(
[(result.binary_package_release, result.archive, result.day,
result.country, result.count) for result in results],
- key=lambda r: (r[0].id, r[2], r[3].name if r[3] else None)))
+ key=lambda r: (
+ r[0].id, r[2],
+ r[3] is not None, r[3].name if r[3] else None)))
diff --git a/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt b/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt
index 3246cca..4b3ca4e 100644
--- a/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt
+++ b/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt
@@ -42,8 +42,10 @@ Celso Providelo PPA builds can be browsed via the API.
An entry can be selected in the returned collection.
+ >>> from operator import itemgetter
>>> from lazr.restful.testing.webservice import pprint_entry
- >>> pprint_entry(sorted(ppa_builds['entries'])[0])
+ >>> pprint_entry(
+ ... sorted(ppa_builds['entries'], key=itemgetter('title'))[0])
arch_tag: 'hppa'
...
title: 'hppa build of mozilla-firefox 0.9 in ubuntu warty RELEASE'
diff --git a/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt b/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt
index 761b7aa..92e2764 100644
--- a/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt
+++ b/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt
@@ -292,7 +292,7 @@ binaries have been copied and published in the same context archive.
>>> source_pub = pubs['entries'][0]
>>> builds = webservice.named_get(
... source_pub['self_link'], 'getBuilds').jsonBody()
- >>> for entry in sorted(builds['entries']):
+ >>> for entry in builds['entries']:
... print(entry['title'])
i386 build of pmount 0.1-1 in ubuntu warty RELEASE
@@ -310,7 +310,7 @@ of that publication.
>>> source_pub = pubs['entries'][0]
>>> builds = webservice.named_get(
... source_pub['self_link'], 'getPublishedBinaries').jsonBody()
- >>> for entry in sorted(builds['entries']):
+ >>> for entry in builds['entries']:
... print(entry['display_name'])
pmount 0.1-1 in warty hppa
pmount 0.1-1 in warty i386
diff --git a/lib/lp/translations/utilities/translationmerger.py b/lib/lp/translations/utilities/translationmerger.py
index 192c32b..7164395 100644
--- a/lib/lp/translations/utilities/translationmerger.py
+++ b/lib/lp/translations/utilities/translationmerger.py
@@ -282,8 +282,16 @@ class MessageSharingMerge(LaunchpadScript):
class_count = len(equivalence_classes)
log.info("Merging %d template equivalence classes." % class_count)
+ def equivalence_class_sort_key(name):
+ template_name, package_name = name
+ if package_name is None:
+ return template_name, 0, None
+ else:
+ return template_name, 1, package_name
+
tm = TransactionManager(self.txn, self.options.dry_run)
- for number, name in enumerate(sorted(equivalence_classes)):
+ for number, name in enumerate(sorted(
+ equivalence_classes, key=equivalence_class_sort_key)):
templates = equivalence_classes[name]
log.info(
"Merging equivalence class '%s': %d template(s) (%d / %d)" % (
