Add chrono support
Moved from https://github.com/chronotope/chrono/pull/542
What's left
- [x] port tests
- [ ] wait for chrono upstream to accept the changes https://github.com/chronotope/chrono/pull/812
cc @Psykopear I just pushed so that you can help review even though it's not complete yet, but all the types are now there
That's great, thank you! A couple of things:
- You mention the need for an update on the
timecrate, to get theSECS_PER_DAYconst, but the const is defined insidechronoitself (insrc/time_delta.rs), which doesn't depend ontimeanymore. It's not public, but we can either just change that, or define it here, it's a fixed number and I hope it doesn't change (not being ironic, I just think I can't assume anything when talking about datetimes) - What you do here for the conversion can be done with the current
chronoapi, it's less efficient (usingdate.year()ecc., rather than exposingdate.mdf()), but it would work without any change tochrono. It might make sense to make this PR work with the current version ofchrono, and maybe update it oncechronoexposes the needed api? I say this because I feel like the changes onchronowould not be trivial, they'd need to expose some structs/functions that are currently private, and this might be a long process. Also, it's should be an implementation detail here, updating it later won't change anything in the exposed functions/structs.
edit: I'll make a proper review of the code tomorrow morning, I'll also try it on the library I'm working on at the moment, where I have some tests for the python integration, and let you know if I have any problem
@pickfire I opened a PR https://github.com/pickfire/pyo3/pull/1 over your fork, I updated this to work with the published version of chrono, without any change to the api. Let me know what you think, and if you like the idea feel free to merge it.