What is the best way to do the following?
use chrono::DateTime;
use pyo3::types::PyDateTime;
let chrono_datetime = DateTime::Utc::now();
let pydatetime: PyDateTime = chrono_datetime.into();
This obviously doesn't work now, and currently doing silly things to get the same effect. Most the the problem is getting chrono::offset::Utc into a PyTzInfo for PyDateTime::from_timestamp
There must be a better way. If not, I'd be happy to provide a PR (with a chrono feature flag(?)) with some pointers on where to start.
Hi @milesgranger, good question!
This would absolutely be welcome as a PR - you would have to implement FromPyObject, IntoPy<PyObject> and ToPyObject for the chrono types. It would be great to have this behind a feature flag.
There's a few recent PRs which added similar conversions for HashSet - you could look at those (or just the implementation in the code) if you want some templates to get started.
Any questions just pop them on here or open a WIP review and I can comment directly on the code.
@milesgranger
Thank you for your interest.
Since chrono is a major library, I think it's worth supporting.
We have some conditional implementation for conversion.
For example, please take a glance at complex coversion.
I guess I can begrudgingly accept why you'd want to do this, but is there any way this can be a third-party library providing this trait instead, or does it have to be provided in one of chrono or pyo3 (I can never remember the rule on this).
Perhaps irrationally, I oppose this because in my experience chrono is a terrible datetime library, and I'm shocked that people seem to have mostly standardized on it. Perhaps it's improved since I last used it a year or two ago.
I'm also not sure how well it would naturally translate to PyDateTime; the non-UTC timezone-aware chrono types would have to use fixed offset zones (which is not a great idiom in Python), and I'm not really clear on what to do with DateTime<Local> - translate it into an aware timezone with a fixed offset, or a naive time?
@pganssle : it can be provided in pyo3 by relying optionally on chrono, which means if you're using pyo3 but not chrono it will not compile the conversion code.
@pganssle : it can be provided in
pyo3by relying optionally onchrono, which means if you're usingpyo3but notchronoit will not compile the conversion code.
I'm not concerned about taking on a dependency on chrono (though yeah we definitely should not), I'm concerned about the maintenance burden of supporting chrono in pyo3. Partially because I think it sets a bad precedent for all the cross-compatibility stuff with third party libraries to live in pyo3 directly rather than in compatibility crates, and partially because I think chrono is a bad datetime library and my hope is that it will eventually be supplanted with something that is actually good (in which case we're left maintaining bindings for some hopefully deprecated old datetime library).
Interesting to see the pitfalls of doing this. Should we rather focus on making the API of things like PyTzInfo more friendly? Thus converting something like chrono::DateTime (or any other datetime lib/obj) to PyDateTime trivial w/ PyDateTime::from_timestamp.
I'm also not sure how well it would naturally translate to PyDateTime; the non-UTC timezone-aware chrono types would have to use fixed offset zones (which is not a great idiom in Python), and I'm not really clear on what to do with DateTime
- translate it into an aware timezone with a fixed offset, or a naive time?
Interesting. Sorry for my ignorance about that.
Should we rather focus on making the API of things like PyTzInfo more friendly?
TzInfo is meant to be used with extends like this example:
#[pyclass(extends=PyTzInfo)]
pub struct TzClass {}
#[pymethods]
impl TzClass {
#[new]
fn new() -> Self {
TzClass {}
}
fn utcoffset<'p>(&self, py: Python<'p>, _dt: &PyDateTime) -> PyResult<&'p PyDelta> {
PyDelta::new(py, 0, 3600, 0, true)
}
fn tzname(&self, _py: Python<'_>, _dt: &PyDateTime) -> PyResult<String> {
Ok(String::from("+01:00"))
}
fn dst(&self, _py: Python<'_>, _dt: &PyDateTime) -> PyResult<Option<&PyDelta>> {
Ok(None)
}
}
So it is not actually straightforward to cooperate with chrono, in which TzInfo is just an offset :thinking:
it sets a bad precedent for all the cross-compatibility stuff with third party libraries to live in pyo3 directly rather than in compatibility crates
I kinda agree with this comment, but I don't think we have a solution for that right now. In the future we might be able to do something like serde remote derive to make it possible for separate compatibility crates to exist.
@davidhewitt how would you see this working under the "remote derive" design, such that we get a real chrono::NaiveDate or similar from PyO3 rather than a wrapper newtype?
We can write the wrapper newtype without assistance from PyO3, but it's painful to manually unwrap and wrap across the Python boundary.
To be honest, I haven't put time into thinking about it at all. If anyone wants to take ownership of researching how serde does it and how we might be able to introduce similar to pyO3, I'd be keen to be a sounding board to bounce ideas off.
There seems to be two key points to the remote derive design:
impl Serialize + Deserialize attached to itserde is able to achieve the above (and more) because it allows you to attach attributes to fields within your type: specifically, #[serde(with = "DurationDef")] performs the unwrapping automatically (see: https://serde.rs/remote-derive.html). These attributes are useful hooks for customisation of serialisation and deserialisation when impl Serialize + Deserialize is not an option.
PyO3 relies fully on trait impls for any customisation of method arguments and return values. I'm not aware of any hook mechanism similar to serde's field attributes.
I could imagine an equivalent mechanism where I put a fn f(#[with(pydate_to_naivedate)] date: NaiveDate) attribute on a PyO3 method parameter, which tells the proc macro to invoke the fn pydate_to_naivedate(&PyDate) -> NaiveDate function instead of looking for impl FromPyObject for NaiveDate.
Hmm interesting idea. I think I'd prefer the attribute to be
fn f(
#[pyo3(with=pydate_to_naivedate)] date: NaiveDate
)
... but other than that it's a cool suggestion and would love to see it!