Merge ~cjwatson/launchpad:py3-frozenset-subclassing into launchpad:master

Proposed by Colin Watson on 2020-06-13
Status: Merged
Approved by: Colin Watson on 2020-07-20
Approved revision: 10d48f70516474d84b89bee5d611e14845268fa2
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:py3-frozenset-subclassing
Merge into: launchpad:master
Diff against target: 138 lines (+66/-8)
2 files modified
lib/lp/bugs/model/bug.py (+62/-5)
lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py (+4/-3)
Reviewer Review Type Date Requested Status
Ioana Lasc 2020-06-13 Approve on 2020-07-17
Review via email: mp+385711@code.launchpad.net

Commit message

Fix frozenset subclassing for Python 3

Description of the change

As of Python 3.0, following the fix for https://bugs.python.org/issue1721812, operations on subclasses of frozenset that create new sets do so using the base type, not the subclass type. Refactor the custom sets in lp.bugs.model.bug to cope with this.

To post a comment you must log in.
Ioana Lasc (ilasc) :
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/model/bug.py b/lib/lp/bugs/model/bug.py
2index 98169c6..0a9062a 100644
3--- a/lib/lp/bugs/model/bug.py
4+++ b/lib/lp/bugs/model/bug.py
5@@ -29,6 +29,10 @@ import re
6 from lazr.lifecycle.event import ObjectCreatedEvent
7 from lazr.lifecycle.snapshot import Snapshot
8 import pytz
9+from six.moves.collections_abc import (
10+ Iterable,
11+ Set,
12+ )
13 from sqlobject import (
14 BoolCol,
15 ForeignKey,
16@@ -2259,8 +2263,61 @@ def load_people(*where):
17 need_preferred_email=True)
18
19
20-class BugSubscriberSet(frozenset):
21- """A set of bug subscribers
22+class FrozenSetBasedSet(Set):
23+ """A subclassable immutable set.
24+
25+ On Python 3, we can't simply subclass `frozenset`: set operations such
26+ as union will create a new set, but it will be a plain `frozenset` and
27+ will lack the appropriate custom properties. However, the `Set` ABC
28+ (which is in fact an immutable set; `MutableSet` is a separate ABC)
29+ knows to create new sets using the same class. Take advantage of this
30+ with a trivial implementation of `Set` that backs straight onto a
31+ `frozenset`.
32+
33+ The ABC doesn't implement the non-operator versions of set methods such
34+ as `union`, so do that here, at least for those methods we actually use.
35+ """
36+
37+ def __init__(self, iterable=None):
38+ self._frozenset = (
39+ frozenset() if iterable is None else frozenset(iterable))
40+
41+ def __iter__(self):
42+ return iter(self._frozenset)
43+
44+ def __contains__(self, value):
45+ return value in self._frozenset
46+
47+ def __len__(self):
48+ return len(self._frozenset)
49+
50+ def issubset(self, other):
51+ return self <= self._from_iterable(other)
52+
53+ def issuperset(self, other):
54+ return self >= self._from_iterable(other)
55+
56+ def union(self, *others):
57+ for other in others:
58+ if not isinstance(other, Iterable):
59+ raise NotImplementedError
60+ return self._from_iterable(value for value in chain(self, *others))
61+
62+ def intersection(self, *others):
63+ for other in others:
64+ if not isinstance(other, Iterable):
65+ raise NotImplementedError
66+ return self._from_iterable(
67+ value for value in chain(*others) if value in self)
68+
69+ def difference(self, *others):
70+ other = self._from_iterable([]).union(*others)
71+ return self._from_iterable(
72+ value for value in self if value not in other)
73+
74+
75+class BugSubscriberSet(FrozenSetBasedSet):
76+ """An immutable set of bug subscribers.
77
78 Every member should provide `IPerson`.
79 """
80@@ -2274,8 +2331,8 @@ class BugSubscriberSet(frozenset):
81 return tuple(sorted(self, key=person_sort_key))
82
83
84-class BugSubscriptionSet(frozenset):
85- """A set of bug subscriptions."""
86+class BugSubscriptionSet(FrozenSetBasedSet):
87+ """An immutable set of bug subscriptions."""
88
89 @cachedproperty
90 def sorted(self):
91@@ -2299,7 +2356,7 @@ class BugSubscriptionSet(frozenset):
92 return BugSubscriberSet(load_people(condition))
93
94
95-class StructuralSubscriptionSet(frozenset):
96+class StructuralSubscriptionSet(FrozenSetBasedSet):
97 """A set of structural subscriptions."""
98
99 @cachedproperty
100diff --git a/lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py b/lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py
101index f261fcb..c9565aa 100644
102--- a/lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py
103+++ b/lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py
104@@ -8,6 +8,7 @@ __metaclass__ = type
105 from contextlib import contextmanager
106
107 import six
108+from six.moves.collections_abc import Set
109 from storm.store import Store
110 from testtools.matchers import Equals
111 from zope.component import queryAdapter
112@@ -71,7 +72,7 @@ class TestSubscriptionRelatedSets(TestCaseWithFactory):
113
114 def test_BugSubscriberSet(self):
115 subscriber_set = BugSubscriberSet(self.subscribers_set)
116- self.assertIsInstance(subscriber_set, frozenset)
117+ self.assertIsInstance(subscriber_set, Set)
118 self.assertEqual(self.subscribers_set, subscriber_set)
119 self.assertEqual(self.subscribers_sorted, subscriber_set.sorted)
120
121@@ -82,7 +83,7 @@ class TestSubscriptionRelatedSets(TestCaseWithFactory):
122 bug.subscribe(subscriber, subscriber)
123 for subscriber in self.subscribers_set)
124 subscription_set = BugSubscriptionSet(subscriptions)
125- self.assertIsInstance(subscription_set, frozenset)
126+ self.assertIsInstance(subscription_set, Set)
127 self.assertEqual(subscriptions, subscription_set)
128 # BugSubscriptionSet.sorted returns a tuple of subscriptions ordered
129 # by subscribers.
130@@ -102,7 +103,7 @@ class TestSubscriptionRelatedSets(TestCaseWithFactory):
131 product.addSubscription(subscriber, subscriber)
132 for subscriber in self.subscribers_set)
133 subscription_set = StructuralSubscriptionSet(subscriptions)
134- self.assertIsInstance(subscription_set, frozenset)
135+ self.assertIsInstance(subscription_set, Set)
136 self.assertEqual(subscriptions, subscription_set)
137 # StructuralSubscriptionSet.sorted returns a tuple of subscriptions
138 # ordered by subscribers.