Allow naive datetime into chrono DateTime<Local> conversion

[bschoenmaeckers]

This special cases chrono's DateTime<Local> from python conversion to allow naive datetimes. As suggested in #5169 (comment).

ref #5169, #5174

[davidhewitt]

Thanks, this seems like a good convenience for the flexibility. The need to remove the FromPyObject is a bit unfortunate but I don't see a good alternative way to solve it, and I think it's worth doing for users' sake?

[davidhewitt]

Removing this is breaking, but I guess it's unlikely to be a problem? The DateTime conversion is still going to work and I would be quite surprised if users ever tried to extract Local directly.

Regardless we should add a 5178.removed.md entry in the changelog which says this implementation is no longer provided by the chrono-local feature.

[bschoenmaeckers]

You removing this is unfortunate. An other option is to conditionally branch on TypeId. Which in const is nicely optimized away.

Do you prefer the following over removing impl FromPyObject<'_> for Local?

impl<Tz: TimeZone + 'static + for<'py> FromPyObject<'py>> FromPyObject<'_> for DateTime<Tz> {
    fn extract_bound(dt: &Bound<'_, PyAny>) -> PyResult<DateTime<Tz>> {
        let dt = dt.downcast::<PyDateTime>()?;
        let tzinfo = dt.get_tzinfo();

        let tz = if let Some(tzinfo) = tzinfo {
            tzinfo.extract()?
        } else {

            // TODO make this check const when PartialEq is const stable
            #[cfg(feature = "chrono-local")]
            if TypeId::of::<Tz>() == TypeId::of::<Local>() {
                // Safety: Tz is Local, so we can safely transmute Local to Tz.
                let tz = unsafe { std::mem::transmute(Local) };
                return py_datetime_to_datetime_with_timezone(dt, tz);
            }

            return Err(PyTypeError::new_err(
                "expected a datetime with non-None tzinfo",
            ));
        };

        py_datetime_to_datetime_with_timezone(dt, tz)
    }
}

[bschoenmaeckers]

Never mind, this does not work in practice.

error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> src\conversions\chrono.rs:341:35
    |
341 |                 let tz = unsafe { std::mem::transmute(Local) };
    |                                   ^^^^^^^^^^^^^^^^^^^
    |
    = note: source type: `Local` (0 bits)
    = note: target type: `Tz` (this type does not have a fixed size)

[davidhewitt]

One option might be to add a hidden specialization to the FromPyObject trait similar to #5244

Though if we go that way, probably we should land this in 0.27 when I think we'll land the rest of the FromPyObject changes.

[bschoenmaeckers]

Never mind, this does not work in practice.

error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> src\conversions\chrono.rs:341:35
    |
341 |                 let tz = unsafe { std::mem::transmute(Local) };
    |                                   ^^^^^^^^^^^^^^^^^^^
    |
    = note: source type: `Local` (0 bits)
    = note: target type: `Tz` (this type does not have a fixed size)

transmute_copy does work here. I wander if there is a safe api to do the TypeId::of check and transmute safely that optimises nicely 🤔.