search

JuliaPy / PythonCall.jl

Python and Julia in harmony.
https://juliapy.github.io/PythonCall.jl/stable/
MIT License
722 stars 61 forks source link

Add support for datetime64 #334

Open hhaensel opened 1 year ago

hhaensel commented 1 year ago

This PR tentatively addresses #293

github-actions[bot] commented 10 months ago

Thank you for your pull request. It has been marked as stale because it has been open for 60 days with no activity. If the PR is still relevant then please leave a comment, or else it will be closed in 30 days.

hhaensel commented 10 months ago

I think, this is still relevant. @cjdoris if you think some modifications are necessary, please let me know.

cjdoris commented 9 months ago

Thanks for the PR! A couple of comments:

PythonCall avoids relying on anything other than the Python standard library - so Py(::Period) should not be converted to a numpy.timedelta64 (if anything it should be a datetime.timdelta). No objection to the reverse conversion though.

However, there are differing semantics going on. Both datetime.timedelta and numpy.timedelta64 represent fixed periods of time, so 1 hour is always 3600 seconds. However a Julia Dates.Period represents a calendar period of time, whose precise length is context dependent, so for example 1 hour can be 3600 or 3601 seconds depending on if the hour has a leap second or not. Therefore it doesn't really make sense to convert between them. It might be OK to convert small units (seconds, milliseconds, etc) because these have a fixed length but (a) this feels like a special case and (b) maybe one day we'll add leap milliseconds to the calendar.

hhaensel commented 9 months ago

PythonCall avoids relying on anything other than the Python standard library - so Py(::Period) should not be converted to a numpy.timedelta64 (if anything it should be a datetime.timdelta). No objection to the reverse conversion though.

Sounds like a good proposal.

However a Julia Dates.Period represents a calendar period of time

I see, I was not aware of the Period semantics in Julia, but it totally makes sense. I think as soon as we leave out Year everything should be fine. At least Julia converts happily between the lower units Week, Hour, Day, Second, etc ... while it throws an error as soon as Year is involved.

Should I wait for the refactoring, before continuing with this PR?