pitrou commented on code in PR #5070:
URL: https://github.com/apache/arrow-rs/pull/5070#discussion_r1391033847


##########
arrow/src/pyarrow.rs:
##########
@@ -118,8 +118,39 @@ fn validate_class(expected: &str, value: &PyAny) -> 
PyResult<()> {
     Ok(())
 }
 
+fn validate_pycapsule(capsule: &PyCapsule, name: &str) -> PyResult<()> {
+    let capsule_name = capsule.name()?;
+    if capsule_name.is_none() {
+        return Err(PyValueError::new_err(
+            "Expected schema PyCapsule to have name set.",
+        ));
+    }
+
+    let capsule_name = capsule_name.unwrap().to_str()?;
+    if capsule_name != name {
+        return Err(PyValueError::new_err(format!(
+            "Expected name '{}' in PyCapsule.",
+            name,
+        )));
+    }
+
+    Ok(())
+}
+
 impl FromPyArrow for DataType {
     fn from_pyarrow(value: &PyAny) -> PyResult<Self> {
+        // Newer versions of PyArrow as well as other libraries with Arrow 
data implement this
+        // method, so prefer it over _export_to_c.

Review Comment:
   You probably want to reference 
https://arrow.apache.org/docs/format/CDataInterface/PyCapsuleInterface.html in 
this comment.



##########
arrow-data/src/ffi.rs:
##########
@@ -282,6 +286,27 @@ impl FFI_ArrowArray {
         // If dictionary is not null should be valid for reads of `Self`
         unsafe { self.dictionary.as_ref() }
     }
+
+    /// Create a copy of an existing `FFI_ArrowArray`
+    ///
+    /// As required by the C Data Interface specification, this sets the 
`release` member of `Self`
+    /// to `None`, but without calling the release callback.
+    pub fn copy(&mut self) -> Self {

Review Comment:
   You could call this "move" for clarity.



##########
arrow-data/src/ffi.rs:
##########
@@ -66,8 +66,12 @@ impl Drop for FFI_ArrowArray {
 unsafe impl Send for FFI_ArrowArray {}
 unsafe impl Sync for FFI_ArrowArray {}
 
-// callback used to drop [FFI_ArrowArray] when it is exported
-unsafe extern "C" fn release_array(array: *mut FFI_ArrowArray) {
+/// callback used to drop [FFI_ArrowArray] when it is exported
+///
+/// # Safety
+///
+/// Must be passed a valid [FFI_ArrowArray].
+pub unsafe extern "C" fn release_array(array: *mut FFI_ArrowArray) {

Review Comment:
   Agreed this doesn't seem useful.



-- 
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]

Reply via email to