-
Notifications
You must be signed in to change notification settings - Fork 98
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
Update Pyo3 bounds #472
Update Pyo3 bounds #472
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks a lot! Let's hope that the arrow
project publishes a new release soon.
7ebeaa8
to
a62eeca
Compare
@haixuanTao the last deprecation warning was here. dora/binaries/runtime/src/operator/python.rs Lines 187 to 197 in bbabdc3
I think that the new bounds API makes this unnecessary, and the deprecation warning states "code not using the I included that change as the last commit, so if you find that it is still necessary it'll be an easy rollback. |
That makes sense, thanks! |
This fix an issue that I had about stopping a dataflow that has been present for a bit! Thanks a lot! Can't wait to merge it. |
@haixuanTao - It could be a few months before If you don't mind pointing to |
If we merge it, we will not be able to publish on cargo, which will makes our release stuck. So I wouldn´t do it. What we could do, is to only merge arrow version within dora-node-api-python that is distributed using pip which does not check for git packages and distribute dora-node-api-python with the latest arrow. But not sure if we can handle multi arrow versions. |
20c7803
to
dbdb38d
Compare
Arrow needs to be upgraded alongside pyo3 because they both link to python. NOTE: Arrow marked `ffi::from_ffi` unsafe. apache/arrow-rs#5080
The argument was removed. See: apache/arrow-rs#5485
dbdb38d
to
1201128
Compare
The deprecation warning states: > code not using the `GIL Refs` API can safely remove the use of `Py::new_pool` All GIL Refs usage have been removed, so this should be fine.
1201128
to
af332c1
Compare
@haixuanTao This is rebased and ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome!
Thanks a million!
pyo3 0.21 is in
arrow-rs
master (see apache/arrow-rs#5566), but not yet released.Bound
API ofpyO3
#437Notes
After updating the deps, this was completely a compiler / clippy driven refactor where clippy highlights deprecation warnings, and I would update using the migration guide.
I did not go looking for any other opportunities to use the new
Bounds
api.