Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make DateTimeRange hashable #39

Open
gdalmau opened this issue Sep 21, 2021 · 5 comments
Open

Make DateTimeRange hashable #39

gdalmau opened this issue Sep 21, 2021 · 5 comments

Comments

@gdalmau
Copy link

gdalmau commented Sep 21, 2021

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

@thombashi
Copy link
Owner

thombashi commented Sep 25, 2021

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
Copy link
Author

gdalmau commented Sep 27, 2021

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
Copy link
Owner

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
Copy link
Author

gdalmau commented Sep 29, 2021

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants