How to implement PyDateTime From<chrono::DateTime>

[milesgranger]

❓ Question

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.

[davidhewitt]

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.

[kngwyu]

@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.

[pganssle]

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?

[althonos]

@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]

@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.

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).

[milesgranger]

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.

[kngwyu]

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 🤔

[davidhewitt]

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.

[1tgr]

@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.

[davidhewitt]

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.

[1tgr]

There seems to be two key points to the remote derive design:

1. In your crate, you declare to serde the layout of the other crate's type. Serde automatically generates a wrapper type (in your crate) that can have impl Serialize + Deserialize attached to it
2. At the end of deserialisation, serde automatically maps the wrapper type back to the original type from the other crate

serde 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.

[davidhewitt]

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!

[kangalio]

For future visitors: an implementation of a newtype around NaiveDateTime that can be passed to Python

EDIT: Use the pyo3-chrono crate instead, which includes conversions for other data types, Python->Rust conversions, and is leap-second aware.

use chrono::{Datelike as _, Timelike as _};

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct DateTime(pub chrono::NaiveDateTime);

impl pyo3::ToPyObject for DateTime {
	fn to_object(&self, py: pyo3::Python) -> pyo3::PyObject {
		pyo3::types::PyDateTime::new(
			py,
			self.0.year(),
			self.0.month() as u8,
			self.0.day() as u8,
			self.0.hour() as u8,
			self.0.minute() as u8,
			self.0.second() as u8,
			self.0.timestamp_subsec_micros(),
			None,
		)
		.unwrap()
		.to_object(py)
	}
}

impl pyo3::IntoPy<pyo3::PyObject> for DateTime {
	fn into_py(self, py: pyo3::Python) -> pyo3::PyObject {
		pyo3::ToPyObject::to_object(&self, py)
	}
}

[utapyngo]

@kangalioo, could you please add the FromPyObject implementation?

Here is mine:

impl<'source> FromPyObject<'source> for DateTime {
    fn extract(ob: &'source PyAny) -> PyResult<Self> {
        let d = PyDateTime::try_from(ob)?;
        Ok(Self(NaiveDate::from_ymd(
            d.get_year() as i32, d.get_month() as u32, d.get_day() as u32,
        ).and_hms_micro(
			d.get_hour() as u32, d.get_minute() as u32, d.get_second() as u32,
			d.get_microsecond(),
		)))
    }
}

But I am new to Rust, so the above code could probably be made simpler.

[kangalio]

Okay, I made a crate that includes all conversions to and from NaiveDateTime, NaiveDate, NaiveTime, and Duration. Everything is rigorously tested and documented ^.^

https://crates.io/crates/pyo3-chrono

It doesn't handle timezones because, as far as I can tell, Python has no real understanding of timezones, you can merely "tack on" a timezone to an otherwise naive datetime. This makes for ambiguity when converting from a timezone-aware chrono::DateTime, so I felt it was best to leave that conversion out entirely.

If I'm wrong on this, I'd gladly learn how to unambiguously convert a chrono::DateTime to a Python datetime.

[utapyngo]

Okay, I made a crate that includes all conversions to and from NaiveDateTime, NaiveDate, NaiveTime, and Duration. Everything is rigorously tested and documented ^.^

crates.io/crates/pyo3-chrono

Thank you @kangalioo, it works.

[pickfire]

I am trying to add integration into chrono directly, we can use the internals without having a separate newtype. chronotope/chrono#542

[pickfire]

Now trying to add timezone but realized that timezone bindings are not available yet. #207 Maybe I will also try to contribute to that as well.

[davidhewitt]

We added chrono support in PyO3 0.17, so this is now resolved.

[kangalio]

This means pyo3-chrono should be deprecated, right?

[davidhewitt]

Yes, I believe we now cover all the functionality in pyo3.

[qmihau]

In the implementation of FromPyObject for DateTime<FixedOffset> (conversions/chrono.rs line 251), shouldn't the return be based on DateTime::from_local() with tz extracted from passed argument, and not DateTime::from_utc()? If I'm passing a datetime.datetime with a non-UTC timezone, grabbing time from it, would result in non-UTC time.

When I started using that, I noticed I can't really reconcile results. For example, if I pass pandas.Timestamp("2023-02-23 13:30:00-0600") and do let local_dt: DateTime<FixedOffset> = value.extract().unwrap(); I get 2023-02-23T07:30:00-06:00, which seems like it takes local time in "-0600" assumes it's UTC and unnecessarily convertes it again to "-0600".

Hopefully I didn't miss anything important that would explain those odd results.

When I looked at tests for that, they follow the same logic, so they test if extract() worked, but not really if the logic is correct.

[adamreichold]

I would glad if you could open a new issue or better yet a pull request detailing the changes to the current implementing and its tests. Additional comments on old issues make it significantly harder for us to keep things organized. And a concrete proposal for code changes is also often a good starting for further discussion.

[qmihau]

Sure, happy to