Jefffrey commented on code in PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#discussion_r1544002230
##########
arrow/src/pyarrow.rs:
##########
@@ -102,8 +102,8 @@ impl<T: ToPyArrow> IntoPyArrow for T {
}
fn validate_class(expected: &str, value: &PyAny) -> PyResult<()> {
- let pyarrow = PyModule::import(value.py(), "pyarrow")?;
- let class = pyarrow.getattr(expected)?;
+ let pyarrow = PyModule::import_bound(value.py(), "pyarrow")?;
+ let class = pyarrow.getattr(expected)?.into_gil_ref(); // TODO
Review Comment:
I'm not familiar with pyo3, so I was following the migration guide:
https://pyo3.rs/v0.21.0/migration.html
Of particular note is:
> PyO3 0.21 introduces a new `Bound<'py, T>` smart pointer which replaces
the existing "GIL Refs" API to interact with Python objects. For example, in
PyO3 0.20 the reference `&'py PyAny` would be used to interact with Python
objects. In PyO3 0.21 the updated type is `Bound<'py, PyAny>`.
So it seems they are moving away from `&PyAny` is my understanding. This
raises a question of if the PyArrow API's should follow suit?
e.g. here, introduce a new function that uses `Bound`?
https://github.com/apache/arrow-rs/blob/5f639061d4880716e458fe1bcce70495054a40f3/arrow/src/pyarrow.rs#L141
For now I've used `into_gil_ref()` to get around this deprecation process,
but I don't think it's meant to be a permanent solution.
I'll do some more reading, maybe it's been discussed in pyo3 before
somewhere :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
