Merge ~cjwatson/launchpad:py3-orderable into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 939df80ba6f7981be9c1d3b228689eccf2387603
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:py3-orderable
Merge into: launchpad:master
Diff against target: 324 lines (+66/-30)
16 files modified
lib/lp/bugs/browser/bugtask.py (+7/-3)
lib/lp/bugs/browser/tests/person-bug-views.txt (+1/-1)
lib/lp/bugs/doc/externalbugtracker-sourceforge.txt (+3/-1)
lib/lp/bugs/model/tests/test_bugtask.py (+9/-6)
lib/lp/bugs/stories/webservice/xx-bug.txt (+5/-3)
lib/lp/bugs/subscribers/bug.py (+2/-1)
lib/lp/code/doc/revision.txt (+3/-1)
lib/lp/code/model/branchmergeproposal.py (+3/-2)
lib/lp/registry/stories/webservice/xx-person.txt (+4/-2)
lib/lp/registry/tests/test_distroseriesdifferencecomment.py (+1/-1)
lib/lp/services/doc/orderingcheck.txt (+7/-3)
lib/lp/services/webservice/stories/multiversion.txt (+4/-1)
lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py (+3/-1)
lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt (+3/-1)
lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt (+2/-2)
lib/lp/translations/utilities/translationmerger.py (+9/-1)
Reviewer Review Type Date Requested Status
Cristian Gonzalez (community) Approve
Review via email: mp+398876@code.launchpad.net

Commit message

Fix sorting of unorderable objects on Python 3

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Cristian Gonzalez (cristiangsp) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py
2index a9d9953..2a139c0 100644
3--- a/lib/lp/bugs/browser/bugtask.py
4+++ b/lib/lp/bugs/browser/bugtask.py
5@@ -690,10 +690,14 @@ class BugTaskView(LaunchpadView, BugViewMixin, FeedsMixin):
6 """
7 event_groups = self._event_groups
8
9+ def activity_sort_key(activity):
10+ target = activity.target
11+ return (
12+ activity.datechanged, 0 if target is None else 1, target,
13+ activity.attribute)
14+
15 def group_activities_by_target(activities):
16- activities = sorted(
17- activities, key=attrgetter(
18- "datechanged", "target", "attribute"))
19+ activities = sorted(activities, key=activity_sort_key)
20 return [
21 {"target": target, "activity": list(activity)}
22 for target, activity in groupby(
23diff --git a/lib/lp/bugs/browser/tests/person-bug-views.txt b/lib/lp/bugs/browser/tests/person-bug-views.txt
24index 439512d..5f1d5ab 100644
25--- a/lib/lp/bugs/browser/tests/person-bug-views.txt
26+++ b/lib/lp/bugs/browser/tests/person-bug-views.txt
27@@ -356,7 +356,7 @@ particular bug (see bug 1357):
28
29 >>> commented_bugtasks_view = create_view(no_priv, '+commentedbugs')
30 >>> commented_bugs = list(commented_bugtasks_view.search().batch)
31- >>> [bugtask.bug.id for bugtask in sorted(commented_bugs)]
32+ >>> [bugtask.bug.id for bugtask in commented_bugs]
33 [1, 1, 1]
34
35
36diff --git a/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt b/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt
37index a01dbdc..bcaf316 100644
38--- a/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt
39+++ b/lib/lp/bugs/doc/externalbugtracker-sourceforge.txt
40@@ -215,9 +215,11 @@ been checked.
41
42 >>> transaction.commit()
43
44+ >>> from operator import attrgetter
45 >>> with sourceforge.responses(trace_calls=True):
46 ... bug_watch_updater.updateBugWatches(
47- ... sourceforge, sorted(example_bug_tracker.watches))
48+ ... sourceforge,
49+ ... sorted(example_bug_tracker.watches, key=attrgetter('id')))
50 INFO Updating 1 watches for 1 bugs on http://bugs.some.where
51 GET http://bugs.some.where/support/tracker.php?aid=1722251
52
53diff --git a/lib/lp/bugs/model/tests/test_bugtask.py b/lib/lp/bugs/model/tests/test_bugtask.py
54index 527120e..21a91e4 100644
55--- a/lib/lp/bugs/model/tests/test_bugtask.py
56+++ b/lib/lp/bugs/model/tests/test_bugtask.py
57@@ -11,7 +11,11 @@ import unittest
58
59 from lazr.lifecycle.snapshot import Snapshot
60 from storm.store import Store
61-from testtools.matchers import Equals
62+from testtools.matchers import (
63+ Equals,
64+ MatchesSetwise,
65+ MatchesStructure,
66+ )
67 from testtools.testcase import ExpectedException
68 import transaction
69 from zope.component import getUtility
70@@ -199,12 +203,11 @@ class TestBugTaskCreation(TestCaseWithFactory):
71 bug_many, mark,
72 [evolution, a_distro, warty],
73 status=BugTaskStatus.FIXRELEASED)
74- tasks = [(t.product, t.distribution, t.distroseries) for t in taskset]
75- tasks.sort()
76
77- self.assertEqual(tasks[0][2], warty)
78- self.assertEqual(tasks[1][1], a_distro)
79- self.assertEqual(tasks[2][0], evolution)
80+ self.assertThat(taskset, MatchesSetwise(
81+ MatchesStructure.byEquality(product=evolution),
82+ MatchesStructure.byEquality(distribution=a_distro),
83+ MatchesStructure.byEquality(distroseries=warty)))
84
85 def test_accesspolicyartifacts_updated(self):
86 # createManyTasks updates the AccessPolicyArtifacts related
87diff --git a/lib/lp/bugs/stories/webservice/xx-bug.txt b/lib/lp/bugs/stories/webservice/xx-bug.txt
88index 04480a5..6121737 100644
89--- a/lib/lp/bugs/stories/webservice/xx-bug.txt
90+++ b/lib/lp/bugs/stories/webservice/xx-bug.txt
91@@ -312,9 +312,11 @@ Bug tasks
92 Each bug may be associated with one or more bug tasks. Much of the
93 data in a bug task is derived from the bug.
94
95+ >>> from operator import itemgetter
96 >>> bug_one_bugtasks_url = bug_one['bug_tasks_collection_link']
97 >>> bug_one_bugtasks = sorted(webservice.get(
98- ... bug_one_bugtasks_url).jsonBody()['entries'])
99+ ... bug_one_bugtasks_url).jsonBody()['entries'],
100+ ... key=itemgetter('self_link'))
101 >>> len(bug_one_bugtasks)
102 3
103
104@@ -825,7 +827,6 @@ Bug subscriptions
105
106 We can get the collection of subscriptions to a bug.
107
108- >>> from operator import itemgetter
109 >>> bug_one_subscriptions_url = bug_one['subscriptions_collection_link']
110 >>> subscriptions = webservice.get(bug_one_subscriptions_url).jsonBody()
111 >>> subscription_entries = sorted(
112@@ -1056,7 +1057,8 @@ case aspects of the bugtask (like status) are slaved to the remote bug
113 report described by the bugwatch.
114
115 >>> bug_one_bug_watches = sorted(webservice.get(
116- ... bug_one['bug_watches_collection_link']).jsonBody()['entries'])
117+ ... bug_one['bug_watches_collection_link']).jsonBody()['entries'],
118+ ... key=itemgetter('self_link'))
119 >>> len(bug_one_bug_watches)
120 4
121
122diff --git a/lib/lp/bugs/subscribers/bug.py b/lib/lp/bugs/subscribers/bug.py
123index 11851eb..d88d27b 100644
124--- a/lib/lp/bugs/subscribers/bug.py
125+++ b/lib/lp/bugs/subscribers/bug.py
126@@ -15,6 +15,7 @@ __all__ = [
127
128
129 import datetime
130+from operator import attrgetter
131
132 from zope.security.proxy import removeSecurityProxy
133
134@@ -227,7 +228,7 @@ def send_bug_details_to_new_bug_subscribers(
135 recipients = bug.getBugNotificationRecipients()
136
137 bug_notification_builder = BugNotificationBuilder(bug, event_creator)
138- for to_person in sorted(to_persons):
139+ for to_person in sorted(to_persons, key=attrgetter('id')):
140 reason, rationale = recipients.getReason(
141 str(removeSecurityProxy(to_person).preferredemail.email))
142 subject, contents = generate_bug_add_email(
143diff --git a/lib/lp/code/doc/revision.txt b/lib/lp/code/doc/revision.txt
144index 46f0219..e514d69 100644
145--- a/lib/lp/code/doc/revision.txt
146+++ b/lib/lp/code/doc/revision.txt
147@@ -112,7 +112,9 @@ sequence attribute is None.
148 >>> ancestry = IStore(BranchRevision).find(
149 ... BranchRevision, BranchRevision.branch == branch)
150 >>> for branch_revision in sorted(ancestry,
151- ... key=lambda r:(r.sequence, r.revision.id), reverse=True):
152+ ... key=lambda r: (
153+ ... 0 if r.sequence is None else 1, r.sequence, r.revision.id),
154+ ... reverse=True):
155 ... print(branch_revision.sequence, branch_revision.revision.id)
156 6 9
157 5 8
158diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py
159index 061fb1e..55a9197 100644
160--- a/lib/lp/code/model/branchmergeproposal.py
161+++ b/lib/lp/code/model/branchmergeproposal.py
162@@ -1290,9 +1290,10 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
163
164 def getRevisionsSinceReviewStart(self):
165 """Get the grouped revisions since the review started."""
166+ all_comments = list(self.all_comments)
167 entries = [
168- ((comment.date_created, -1), comment) for comment
169- in self.all_comments]
170+ ((comment.date_created, i - len(all_comments)), comment)
171+ for i, comment in enumerate(all_comments)]
172 entries.extend(self._getNewerRevisions())
173 entries.sort()
174 current_group = []
175diff --git a/lib/lp/registry/stories/webservice/xx-person.txt b/lib/lp/registry/stories/webservice/xx-person.txt
176index 73450d0..619784c 100644
177--- a/lib/lp/registry/stories/webservice/xx-person.txt
178+++ b/lib/lp/registry/stories/webservice/xx-person.txt
179@@ -303,8 +303,10 @@ Team memberships are first-class objects with their own URLs.
180
181 Team memberships also have data fields.
182
183- >>> salgado_landscape = sorted(webservice.get(
184- ... salgado_memberships).jsonBody()['entries'])[1]
185+ >>> salgado_landscape = [
186+ ... entry for entry in webservice.get(
187+ ... salgado_memberships).jsonBody()['entries']
188+ ... if entry['team_link'].endswith('~landscape-developers')][0]
189 >>> for key in sorted(salgado_landscape):
190 ... print(key)
191 date_expires
192diff --git a/lib/lp/registry/tests/test_distroseriesdifferencecomment.py b/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
193index f95aac7..6276647 100644
194--- a/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
195+++ b/lib/lp/registry/tests/test_distroseriesdifferencecomment.py
196@@ -33,7 +33,7 @@ def get_comment_source():
197
198 def randomize_list(original_list):
199 """Sort a list (or other iterable) in random order. Return list."""
200- return sorted(original_list, key=lambda _: random)
201+ return sorted(original_list, key=lambda _: random())
202
203
204 class DistroSeriesDifferenceCommentTestCase(TestCaseWithFactory):
205diff --git a/lib/lp/services/doc/orderingcheck.txt b/lib/lp/services/doc/orderingcheck.txt
206index 209b6e0..e2367aa 100644
207--- a/lib/lp/services/doc/orderingcheck.txt
208+++ b/lib/lp/services/doc/orderingcheck.txt
209@@ -10,8 +10,12 @@ seem worth the trouble.
210 >>> from lp.services.orderingcheck import OrderingCheck
211
212 >>> def sort_key(item):
213- ... """Simple sorting key for integers: the integer itself."""
214- ... return item
215+ ... """Simple sorting key for integers.
216+ ...
217+ ... This could just be the integer itself, but in order to support
218+ ... None on Python 3 we need an additional term.
219+ ... """
220+ ... return (0 if item is None else 1, item)
221
222 The OrderingCheck makes it clean and easy. You create an OrderingCheck
223 with the same arguments that go into Python's standard sorting
224@@ -51,7 +55,7 @@ error.
225 Edge cases
226 ----------
227
228-It is safe to use the None value. Python places it below any other
229+It is safe to use the None value. sort_key places it below any other
230 integer.
231
232 >>> checker = OrderingCheck(key=sort_key)
233diff --git a/lib/lp/services/webservice/stories/multiversion.txt b/lib/lp/services/webservice/stories/multiversion.txt
234index 22f9722..683325f 100644
235--- a/lib/lp/services/webservice/stories/multiversion.txt
236+++ b/lib/lp/services/webservice/stories/multiversion.txt
237@@ -46,11 +46,14 @@ In the 'beta' version of the web service, mutator methods like
238 IBugTask.transitionToStatus are published as named operations. In
239 subsequent versions, those named operations are not published.
240
241+ >>> from operator import itemgetter
242+
243 >>> def get_bugtask_path(version):
244 ... bug_one = webservice.get("/bugs/1", api_version=version).jsonBody()
245 ... bug_one_bugtasks_url = bug_one['bug_tasks_collection_link']
246 ... bug_one_bugtasks = sorted(webservice.get(
247- ... bug_one_bugtasks_url).jsonBody()['entries'])
248+ ... bug_one_bugtasks_url).jsonBody()['entries'],
249+ ... key=itemgetter('self_link'))
250 ... bugtask_path = bug_one_bugtasks[0]['self_link']
251 ... return bugtask_path
252
253diff --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
254index 0b82990..187323c 100644
255--- a/lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py
256+++ b/lib/lp/soyuz/scripts/tests/test_ppa_apache_log_parser.py
257@@ -152,4 +152,6 @@ class TestScriptRunning(TestCaseWithFactory):
258 sorted(
259 [(result.binary_package_release, result.archive, result.day,
260 result.country, result.count) for result in results],
261- key=lambda r: (r[0].id, r[2], r[3].name if r[3] else None)))
262+ key=lambda r: (
263+ r[0].id, r[2],
264+ r[3] is not None, r[3].name if r[3] else None)))
265diff --git a/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt b/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt
266index 3246cca..4b3ca4e 100644
267--- a/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt
268+++ b/lib/lp/soyuz/stories/webservice/xx-hasbuildrecords.txt
269@@ -42,8 +42,10 @@ Celso Providelo PPA builds can be browsed via the API.
270
271 An entry can be selected in the returned collection.
272
273+ >>> from operator import itemgetter
274 >>> from lazr.restful.testing.webservice import pprint_entry
275- >>> pprint_entry(sorted(ppa_builds['entries'])[0])
276+ >>> pprint_entry(
277+ ... sorted(ppa_builds['entries'], key=itemgetter('title'))[0])
278 arch_tag: 'hppa'
279 ...
280 title: 'hppa build of mozilla-firefox 0.9 in ubuntu warty RELEASE'
281diff --git a/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt b/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt
282index 761b7aa..92e2764 100644
283--- a/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt
284+++ b/lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt
285@@ -292,7 +292,7 @@ binaries have been copied and published in the same context archive.
286 >>> source_pub = pubs['entries'][0]
287 >>> builds = webservice.named_get(
288 ... source_pub['self_link'], 'getBuilds').jsonBody()
289- >>> for entry in sorted(builds['entries']):
290+ >>> for entry in builds['entries']:
291 ... print(entry['title'])
292 i386 build of pmount 0.1-1 in ubuntu warty RELEASE
293
294@@ -310,7 +310,7 @@ of that publication.
295 >>> source_pub = pubs['entries'][0]
296 >>> builds = webservice.named_get(
297 ... source_pub['self_link'], 'getPublishedBinaries').jsonBody()
298- >>> for entry in sorted(builds['entries']):
299+ >>> for entry in builds['entries']:
300 ... print(entry['display_name'])
301 pmount 0.1-1 in warty hppa
302 pmount 0.1-1 in warty i386
303diff --git a/lib/lp/translations/utilities/translationmerger.py b/lib/lp/translations/utilities/translationmerger.py
304index 192c32b..7164395 100644
305--- a/lib/lp/translations/utilities/translationmerger.py
306+++ b/lib/lp/translations/utilities/translationmerger.py
307@@ -282,8 +282,16 @@ class MessageSharingMerge(LaunchpadScript):
308 class_count = len(equivalence_classes)
309 log.info("Merging %d template equivalence classes." % class_count)
310
311+ def equivalence_class_sort_key(name):
312+ template_name, package_name = name
313+ if package_name is None:
314+ return template_name, 0, None
315+ else:
316+ return template_name, 1, package_name
317+
318 tm = TransactionManager(self.txn, self.options.dry_run)
319- for number, name in enumerate(sorted(equivalence_classes)):
320+ for number, name in enumerate(sorted(
321+ equivalence_classes, key=equivalence_class_sort_key)):
322 templates = equivalence_classes[name]
323 log.info(
324 "Merging equivalence class '%s': %d template(s) (%d / %d)" % (

Subscribers

People subscribed via source and target branches

to status/vote changes: