Skip to content

KeyBuilder: add datetime hashing#219

Merged
inducer merged 5 commits intoinducer:mainfrom
matthiasdiener:keyb-datetime
Apr 24, 2024
Merged

KeyBuilder: add datetime hashing#219
inducer merged 5 commits intoinducer:mainfrom
matthiasdiener:keyb-datetime

Conversation

@matthiasdiener
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread pytools/persistent_dict.py Outdated
update_for_Map = update_for_frozendict # noqa: N815

def update_for_datetime(self, key_hash: Hash, key: Any) -> None:
self.rec(key_hash, key.isoformat(timespec="microseconds"))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. Two datetimes from different timezones can compare equal. Look at what datetime.__eq__ does? Maybe use timestamp? (And please add a test for the timezone thing.)

@inducer
Copy link
Copy Markdown
Owner

inducer commented Apr 24, 2024

Could you look at the failures here? (Or move this to draft status if it's not ready?)

@matthiasdiener matthiasdiener marked this pull request as draft April 24, 2024 16:19
@matthiasdiener matthiasdiener marked this pull request as ready for review April 24, 2024 17:07
@matthiasdiener
Copy link
Copy Markdown
Contributor Author

Thanks @inducer , I think this is ready for another look

@inducer inducer merged commit a5efddf into inducer:main Apr 24, 2024
@inducer
Copy link
Copy Markdown
Owner

inducer commented Apr 24, 2024

Thx!

@matthiasdiener matthiasdiener deleted the keyb-datetime branch April 24, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants