Skip to content

Commit

Permalink
Merge #877
Browse files Browse the repository at this point in the history
877: UnitContainer: hash collision does not guarantee equality r=hgrecco a=crusaderky

Closes #876

Co-authored-by: Guido Imperiale <guido.imperiale@amphorainc.com>
  • Loading branch information
bors[bot] and Guido Imperiale authored Sep 11, 2019
2 parents d655f32 + 0a84c83 commit d804d93
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 6 deletions.
20 changes: 19 additions & 1 deletion pint/testsuite/test_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from pint.unit import UnitsContainer
from pint.util import ParserHelper

from pint.compat import np, long_type
from pint.compat import np
from pint.errors import UndefinedUnitError, DimensionalityError
from pint.testsuite import QuantityTestCase, helpers

Expand Down Expand Up @@ -699,3 +699,21 @@ def test_issue856b(self):
ureg2.define('test123 = 456 kg')
assert ureg1('1 test123').to('kg').magnitude == 123
assert ureg2('1 test123').to('kg').magnitude == 456

def test_issue876(self):
# Same hash must not imply equality.

# As an implementation detail of CPython, hash(-1) == hash(-2).
# This test is useless in potential alternative Python implementations where
# hash(-1) != hash(-2); one would need to find hash collisions specific for each
# implementation

a = UnitsContainer({"[mass]": -1})
b = UnitsContainer({"[mass]": -2})
c = UnitsContainer({"[mass]": -3})

# Guarantee working on alternative Python implementations
assert (hash(-1) == hash(-2)) == (hash(a) == hash(b))
assert (hash(-1) == hash(-3)) == (hash(a) == hash(c))
assert a != b
assert a != c
19 changes: 14 additions & 5 deletions pint/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,16 @@ def __setstate__(self, state):

def __eq__(self, other):
if isinstance(other, UnitsContainer):
# Not the same as hash(self); see ParserHelper.__hash__ and __eq__
return UnitsContainer.__hash__(self) == UnitsContainer.__hash__(other)
if isinstance(other, string_types):
# UnitsContainer.__hash__(self) is not the same as hash(self); see
# ParserHelper.__hash__ and __eq__.
# Different hashes guarantee that the actual contents are different, but
# identical hashes give no guarantee of equality.
# e.g. in CPython, hash(-1) == hash(-2)
if UnitsContainer.__hash__(self) != UnitsContainer.__hash__(other):
return False
other = other._d

elif isinstance(other, string_types):
other = ParserHelper.from_string(other)
other = other._d

Expand Down Expand Up @@ -504,9 +511,11 @@ def __setstate__(self, state):
self._d, self._hash, self.scale = state

def __eq__(self, other):
if isinstance(other, self.__class__):
return self.scale == other.scale and\
if isinstance(other, ParserHelper):
return (
self.scale == other.scale and
super(ParserHelper, self).__eq__(other)
)
elif isinstance(other, string_types):
return self == ParserHelper.from_string(other)
elif isinstance(other, Number):
Expand Down

0 comments on commit d804d93

Please sign in to comment.