Tpt commented on code in PR #1426:
URL:
https://github.com/apache/datafusion-python/pull/1426#discussion_r2934888946
##########
examples/datafusion-ffi-example/src/utils.rs:
##########
@@ -35,30 +34,10 @@ pub(crate) fn ffi_logical_codec_from_pycapsule(
};
let capsule = capsule.cast::<PyCapsule>()?;
- validate_pycapsule(capsule, "datafusion_logical_extension_codec")?;
-
let data: NonNull<FFI_LogicalExtensionCodec> = capsule
- .pointer_checked(Some(c_str!("datafusion_logical_extension_codec")))?
+ .pointer_checked(Some(c"datafusion_logical_extension_codec"))?
.cast();
let codec = unsafe { data.as_ref() };
Ok(codec.clone())
}
-
-pub(crate) fn validate_pycapsule(capsule: &Bound<PyCapsule>, name: &str) ->
PyResult<()> {
- let capsule_name = capsule.name()?;
- if capsule_name.is_none() {
- return Err(PyValueError::new_err(format!(
- "Expected {name} PyCapsule to have name set."
- )));
- }
-
- let capsule_name = unsafe { capsule_name.unwrap().as_cstr().to_str()? };
- if capsule_name != name {
- return Err(PyValueError::new_err(format!(
- "Expected name '{name}' in PyCapsule, instead got '{capsule_name}'"
- )));
- }
Review Comment:
> Thanks for pointing out the error messages from
[PyCapsule_GetPointer](https://github.com/python/cpython/blob/3c38feb2a21aacdb009eea8baa2a6a3daf4b4932/Objects/capsule.c#L98-L113)!
They do seem fairly similar 😄
I am also contributing to PyO3. It made me move deep in some CPython rabbit
hole.
> having a helper with clearer validation and error messages could make
debugging easier
If we want nice helper what about abstracting as much as possible the
PyCapsule details to get an API like:
```rust
#[pymethods]
impl MyTableProvider {
pub fn __datafusion_table_provider__<'py>(
&self,
py: Python<'py>,
session: PyDataFusionCapsule<Arc<dyn LogicalExtensionCodec>>,
) -> PyResult<PyDataFusionCapsule<Arc<dyn TableProvider>>> {
}
}
```
If you want I can give it a try after #1414 is merged and in the meantime
close this MR. My hope it that it could work by playing around with some traits.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
