thombashi / DateTimeRange

DateTimeRange is a Python library to handle a time range. e.g. check whether a time is within the time range, get the intersection of time ranges, truncate a time range, iterate through a time range, and so forth.
https://datetimerange.rtfd.io/
MIT License
106 stars 16 forks source link

Make DateTimeRange hashable #39

Open gdalmau opened 3 years ago

gdalmau commented 3 years ago

Implement __hash__ function on DateTimeRange to be able to use it, for example, in combination with the decorator @lru_cache.

thombashi commented 3 years ago

Thank you for your feedback.

Implement hash function on DateTimeRange

DateTimeRange is a mutable class. According to the official document, such a class should not implement __hash__(). Therefore, may require a new immutable class like FrozenDateTimeRange.

gdalmau commented 3 years ago

DateTimeRange is a mutable class. According to the official document, such a class should not implement __hash__().

Good point. Since DateTimeRange is composed of two datetime objects, which are immutable, might it be an idea to also make DateTimeRange immutable?

thombashi commented 3 years ago

DateTimeRange is composed of two datetime objects, which are immutable,

This is true. However, I believe DateTimeRange is a mutable class.

Implementation of __hash__ method for DateTimeRange itself is pretty easy. for example:

def __hash__(self):
    return hash((self.start_datetime, self.end_datetime))

This hash output will be change if start_datetime/end_datetime value changes:

from datetimerange import DateTimeRange

time_range = DateTimeRange.from_range_text("2015-03-22T10:00:00+0900 - 2015-03-22T10:10:00+0900")
print(hash(time_range))
time_range.set_end_datetime("2016-03-22T10:10:00+0900")
print(hash(time_range))
-2291305032593123401
5944198212632818997
gdalmau commented 3 years ago

This hash output will be change if start_datetime/end_datetime value changes

I understand, that's why I proposed the idea to make DateTimeRange immutable. For that, set_start_datetime and set_end_datetime and any other setter should return another instance, similar to what datetime.replace() does. If that sounds like an idea, how would you prefer to approach it? We can make use of __slots__ or dataclass with frozen=True.

thombashi commented 3 years ago

I still would prefer to approach that creates an immutable DateTimeRange class as a different class.

set_start_datetime and set_end_datetime and any other setter should return another instance, similar to what datetime.replace() does. If that sounds like an idea, how would you prefer to approach it? We can make use of slots or dataclass with frozen=True.

because these approaches will break the backward compatibility of DateTimeRange class.