diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..3d7a6c1 --- /dev/null +++ b/PR.md @@ -0,0 +1,65 @@ +# Make `Delorean` hashable again + +## Problem + +`Delorean` instances cannot be put in a `set` or used as `dict` keys: + +```python +>>> from datetime import datetime +>>> from delorean import Delorean +>>> d = Delorean(datetime(2015, 1, 1), timezone="UTC") +>>> hash(d) +Traceback (most recent call last): + ... +TypeError: unhashable type: 'Delorean' +>>> {d} +TypeError: unhashable type: 'Delorean' +``` + +This is surprising for a date/time value object: the standard library's +`datetime.datetime` is hashable, and `Delorean` supports equality and ordering, +so grouping instances in sets or keying dictionaries by them is a natural thing +to want. + +## Root cause + +`delorean/dates.py` defines `__eq__` (and the rich-comparison operators) on +`Delorean`, but does not define `__hash__`. In Python 3, defining `__eq__` +without `__hash__` sets `__hash__` to `None`, which makes the class unhashable: + +```python +>>> Delorean.__hash__ is None +True +``` + +Under Python 2 (the version delorean originally targeted) defining `__eq__` +left the inherited `__hash__` in place, so this is effectively a regression +introduced by the move to Python 3. + +## Fix + +Add a `__hash__` consistent with the existing `__eq__`. `__eq__` compares +`self.epoch == other.epoch`, so hashing on `self.epoch` guarantees the hash +invariant (equal objects hash equal), including two instances describing the +same instant in different timezones: + +```python +def __hash__(self): + return hash(self.epoch) +``` + +## Test + +Added two tests to `tests/delorean_tests.py`: + +- `test_hashable`: `hash()` works and an instance can be stored in a set and + used as a dict key. +- `test_hash_consistent_with_equality`: two equal instances (same instant, + different timezone) hash equal and collapse to one element in a set. + +## Gate + +- `python -m pytest tests/` -> 232 passed, 44 subtests passed. + +Counterfactual (fix reverted, tests kept): both new tests fail with +`TypeError: unhashable type: 'Delorean'`. diff --git a/delorean/dates.py b/delorean/dates.py index 4dc00ba..71e08df 100644 --- a/delorean/dates.py +++ b/delorean/dates.py @@ -240,6 +240,13 @@ class Delorean(object): return self.epoch == other.epoch return False + def __hash__(self): + # Defining __eq__ sets __hash__ to None in Python 3, which makes + # Delorean objects unhashable and unusable in sets or as dict keys. + # Hash on the same value equality is based on so that equal objects + # (same instant, regardless of timezone) share a hash. + return hash(self.epoch) + def __lt__(self, other): return self.epoch < other.epoch diff --git a/tests/delorean_tests.py b/tests/delorean_tests.py index 5ab5061..1c0055d 100644 --- a/tests/delorean_tests.py +++ b/tests/delorean_tests.py @@ -50,6 +50,21 @@ class DeloreanTests(unittest.TestCase): dt = date(2013, 5, 6) self.assertRaises(ValueError, delorean.Delorean, dt) + def test_hashable(self): + # Delorean defines __eq__, so it must define __hash__ too, otherwise + # Python 3 makes instances unhashable and unusable in sets/dict keys. + self.assertEqual(hash(self.do), hash(self.do)) + self.assertIn(self.do, {self.do}) + mapping = {self.do: "value"} + self.assertEqual(mapping[self.do], "value") + + def test_hash_consistent_with_equality(self): + # Equal objects (same instant, different timezone) must hash equal. + other = self.do.shift("US/Pacific") + self.assertEqual(self.do, other) + self.assertEqual(hash(self.do), hash(other)) + self.assertEqual(len({self.do, other}), 1) + def test_initialize_from_datetime_naive(self): self.assertRaises( delorean.DeloreanInvalidTimezone, delorean.Delorean, datetime=self.naive_dt