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

Upgrade PyO3 to 0.21 and switch to new API #15215

Closed
10 tasks done
itamarst opened this issue Mar 21, 2024 · 6 comments
Closed
10 tasks done

Upgrade PyO3 to 0.21 and switch to new API #15215

itamarst opened this issue Mar 21, 2024 · 6 comments
Assignees
Labels
accepted Ready for implementation build Changes that affect the build system or external dependencies python Related to Python Polars

Comments

@itamarst
Copy link
Contributor

itamarst commented Mar 21, 2024

Description

In PyO3 0.21, &'py PyAny will be replaced by Bound<'py, PyAny>. The benefits for upgrading are increased performance, the costs of not upgrading are that the old API is deprecated and will, eventually, go away: https://polar.sh/davidhewitt/posts/replacing-pyo3-api-pt1

https://pyo3.rs/latest/migration documents the upgrade process (at time of writing 0.21 is still beta, but will presumably be released soon). Based on their suggestions, I think this would look like this for Polars:

  1. First PR: Switch to 0.21, enable gil-refs feature, fix other migration issues.
  2. A series of PRs that migrate over to Bound. The development process would involve disabling gil-refs temporarily, locally only, to spot APIs that need migration, but given this would probably be a series of PRs, the feature should continue to be on in main.
  3. Final PR: When no deprecation warnings are left, disable gil-refs.

Followups include:

@itamarst itamarst added the enhancement New feature or an improvement of an existing feature label Mar 21, 2024
@itamarst
Copy link
Contributor Author

This is something I can help work on.

@stinodego
Copy link
Member

Yes we will definitely upgrade to the new PyO3 version when it's released. I had planned to work on that myself but if it's something you're interested in working on I think it's fine if you want to do the honors here.

@stinodego stinodego added accepted Ready for implementation internal An internal refactor or improvement build Changes that affect the build system or external dependencies python Related to Python Polars and removed enhancement New feature or an improvement of an existing feature internal An internal refactor or improvement labels Mar 21, 2024
@stinodego stinodego changed the title Switch away from functionality deprecated in PyO3 0.21, in particular &'py PyAny and friends Upgrade PyO3 to 0.21 and switch to new API Mar 21, 2024
@itamarst
Copy link
Contributor Author

itamarst commented Apr 4, 2024

0.21.1 is out, so I will start on this either this week or next.

@stinodego
Copy link
Member

stinodego commented Apr 15, 2024

Something about support for chrono? Need to get reminder from @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.

@itamarst
Copy link
Contributor Author

I'm gonna say this is done, insofar as it covers the main Polars repository, and the pyo3-polars issue is already filed for that repository.

@stinodego
Copy link
Member

Thanks a lot for the thorough work @itamarst !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation build Changes that affect the build system or external dependencies python Related to Python Polars
Projects
Archived in project
Development

No branches or pull requests

2 participants