westonpace commented on code in PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#discussion_r1547901346
##########
arrow/src/pyarrow.rs:
##########
@@ -101,15 +106,19 @@ impl<T: ToPyArrow> IntoPyArrow for T {
}
}
-fn validate_class(expected: &str, value: &PyAny) -> PyResult<()> {
- let pyarrow = PyModule::import(value.py(), "pyarrow")?;
+fn validate_class(expected: &str, value: &Bound<PyAny>) -> PyResult<()> {
+ let pyarrow = PyModule::import_bound(value.py(), "pyarrow")?;
let class = pyarrow.getattr(expected)?;
- if !value.is_instance(class)? {
- let expected_module = class.getattr("__module__")?.extract::<&str>()?;
- let expected_name = class.getattr("__name__")?.extract::<&str>()?;
+ if !value.is_instance(&class)? {
+ let expected_module = class.getattr("__module__")?;
+ let expected_module = expected_module.extract::<&str>()?;
+ let expected_name = class.getattr("__name__")?;
+ let expected_name = expected_name.extract::<&str>()?;
let found_class = value.get_type();
- let found_module =
found_class.getattr("__module__")?.extract::<&str>()?;
- let found_name = found_class.getattr("__name__")?.extract::<&str>()?;
+ let found_module = found_class.getattr("__module__")?;
+ let found_module = found_module.extract::<&str>()?;
+ let found_name = found_class.getattr("__name__")?;
+ let found_name = found_name.extract::<&str>()?;
Review Comment:
This also breaks for me when `gil-refs` is not enabled. From [this
issue](https://github.com/PyO3/pyo3/pull/3928) it appears that `&str` lost
`FromPyObject`:
> Finally, without the gil-refs feature we remove the existing
implementation of FromPyObject for &str and instead implement this new trait.
This works except for abi3 on old Python versions, where we have to settle for
PyFunctionArgument only and allow users of .extract() to break.
>
> This does implement some very targeted breakage for specific
considerations, but I wonder if this is necessary as the correct way to users
off the GIL Ref API in a case where otherwise it's near impossible for them to
realise this uses it.
The reason it is not caught by CI is because CI does not have any `abi...`
feature enabled.
--
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]
