Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to PyO3 0.21's native datetime support, now available with abi3 #16158

Closed
Tracked by #15215
itamarst opened this issue May 10, 2024 · 9 comments · Fixed by #16203
Closed
Tracked by #15215

Switch to PyO3 0.21's native datetime support, now available with abi3 #16158

itamarst opened this issue May 10, 2024 · 9 comments · Fixed by #16203
Assignees

Comments

@itamarst
Copy link
Contributor

Per @stinodego:

PyO3 has support for the Python datetime module:
https://pyo3.rs/main/doc/pyo3/types/struct.pydatetime

However, this was never available under the abi3 feature flag.

So we have a bunch of workarounds in place to materialize/read datetime objects. Specially, we have defined Python functions here:
https://github.com/pola-rs/polars/blob/main/py-polars/polars/_utils/convert.py

...that are called from Rust, for example here:

AnyValue::Date(v) => {
let convert = utils.getattr(intern!(py, "to_py_date")).unwrap();
convert.call1((v,)).unwrap().into_py(py)
},

The 0.21 release notes mention:

Extended chrono / datetime conversions, including support for the abi3 feature and the chrono-tz crate

So I am expecting that we can now take advantage of native PyO3 datetime support. And I expect a good performance boost + code cleanup to come from this.

Originally posted by @stinodego in #15215 (comment)

@itamarst
Copy link
Contributor Author

Gonna do this next.

@itamarst
Copy link
Contributor Author

In addition to the to_py_date() and friends, there's also logic in the opposite direction that might also be possible to remove as part of this.

@itamarst
Copy link
Contributor Author

PyO3/pyo3#3266 means that timezone conversions are lossy, the string timezone is converted to a fixed offset in seconds. So might not be able to convert everything immediately.

@ritchie46
Copy link
Member

Interesting. That saves a lot of sub-optimal back and forth.

@itamarst
Copy link
Contributor Author

Digging further, it may be that the PyO3 limitation is tied to version of Python, and upping minimum supported version to 3.9 will fix it. Python 3.8 security EOL is October 31, 2024...

@itamarst
Copy link
Contributor Author

(Around 5% of Polars downloads are Python 3.8, per https://pypistats.org/packages/polars).

@stinodego
Copy link
Member

FYI, so far we have always kept support for Python versions until their end of life, and then dropped support in the next release. Which I think is a good policy.

So if Python 3.8 support is problematic, this would be blocked until November this year.

@ritchie46
Copy link
Member

Yes, we will follow python in this regard.

@itamarst
Copy link
Contributor Author

itamarst commented May 11, 2024

Ok, will just file a ticket for second pass optimization (#16199).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants