Skip to content

Commit

Permalink
Fix equality under Python3.6.
Browse files Browse the repository at this point in the history
Test failures revealed isinstance for types derived from tuple behaves
unexpectedly under Python3.6. Although this can be worked around by
saving off the mro in __init_subclass__ and using that to perform a
custom isinstance check, more investigation indicated two aspects of
the status quo were undesirable:

1. By returning NotImplemented we left equality up to foreign types
to decide.
2. The intended use of Collection subtypes is for the engine, which
treats all types as distinct; yet we would compare subtypes as equal.

As such implement exact type match based equality and document this.
  • Loading branch information
jsirois committed Jul 26, 2020
1 parent 4cf6c31 commit 2e04a7b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/python/pants/engine/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class Example:
class Examples(Collection[Example]):
pass
N.B: Collection instances are only considered equal if both their types and contents are equal.
"""

@overload # noqa: F811
Expand All @@ -36,9 +38,7 @@ def __getitem__(self, index: Union[int, slice]) -> Union[T, "Collection[T]"]: #
return self.__class__(cast(Tuple[T, ...], result))

def __eq__(self, other: Any) -> bool:
if not isinstance(other, self.__class__):
return NotImplemented
return super().__eq__(other)
return type(self) == type(other) and super().__eq__(other)

def __ne__(self, other: Any) -> bool:
# We must explicitly override to provide the inverse of _our_ __eq__ and not get the
Expand Down
3 changes: 3 additions & 0 deletions src/python/pants/engine/collection_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ def test_collection_reversed() -> None:


def test_collection_equality() -> None:
assert () != Collection()
assert Collection() != ()

assert Collection([]) == Collection([])
c1 = Collection([1, 2, 3])
assert c1 == Collection([1, 2, 3])
Expand Down

0 comments on commit 2e04a7b

Please sign in to comment.