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

Add support for datetime64 and timedelta64 element types #308

Merged
merged 4 commits into from
Apr 14, 2022
Merged

Conversation

adamreichold
Copy link
Member

@adamreichold adamreichold commented Mar 28, 2022

It basically works, but there are still quite a few things left to do:

  • inline documentation (define_units! needs to learn about docstrings)
  • changelog entry
  • more unit tests , integration tests
  • usage in extension examples via pytest
  • how to expose the units? re-export in crate root? put into re-exported units module? make datetime public without re-export? make datetime and within it units public
  • what conversions to/from Datetime and Timedelta to provide? Duration? SystemTime? other crates like chrono?
  • what conversions between different units to provide? handled by PyArray::{copy_to,cast} at the array level
  • whether and if so which operations to provide? none
  • better Debug impls on Datetime and Timedelta

Closes #142

@adamreichold adamreichold force-pushed the datetime branch 7 times, most recently from 6549a5f to 376b6c8 Compare April 6, 2022 20:13
@adamreichold adamreichold marked this pull request as ready for review April 6, 2022 20:20
@adamreichold
Copy link
Member Author

@kngwyu @davidhewitt I think this is ready for review now.

There is one place where I am still undecided though: Whether I should provide conversions to/from SystemTime and Duration. Implementing this generically will be quite a mouthful and it will not be able to cover the whole range provided by NumPy losslessly. Furthermore, actual code needs to decide on a specific unit via a type parameter anyway and this is then a much more tractable problem.

Hence, I am currently leaning against providing any integration with std's time module (or with chrono for that matter).

@kngwyu
Copy link
Member

kngwyu commented Apr 8, 2022

Thank you, the implementation looks nice, but let me a bit investigate how this can work together with PyO3's datatime. I have no experience of using numpy's datetime, and don't know much about its relationship with datetime in Python's std lib.

@adamreichold
Copy link
Member Author

I have no experience of using numpy's datetime, and don't know much about its relationship with datetime in Python's std lib.

Judging from what I learned so far, they seem to be rather different beasts, c.f. https://stackoverflow.com/questions/13703720/converting-between-datetime-timestamp-and-datetime64

So mainly instead of mucking around with calendars, time zones and other such complications, the NumPy datetime types seem to be rather scientifically minded having a huge range depending on the chosen unit but basically just differentiating between absolute (datetime64) and relative (timedelta64) quantities. (For example, converting a year to days is independent of the year itself but uses the average length of years over a 400 year span of the Gregorian calendar. Similarly, a month is always approximately 30.45 days long.)

@kngwyu
Copy link
Member

kngwyu commented Apr 9, 2022

Yeah, I also investigated a bit and found they are not very compatible. Let's launch this PR with some documents added.

@adamreichold adamreichold merged commit de381b6 into main Apr 14, 2022
@adamreichold adamreichold deleted the datetime branch April 14, 2022 17:58
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.

Support array of timedelta64
2 participants