alamb commented on code in PR #8790:
URL: https://github.com/apache/arrow-rs/pull/8790#discussion_r2531912775


##########
arrow-array/src/ffi_stream.rs:
##########
@@ -364,7 +364,13 @@ impl Iterator for ArrowArrayStreamReader {
             let result = unsafe {
                 from_ffi_and_data_type(array, 
DataType::Struct(self.schema().fields().clone()))
             };
-            Some(result.map(|data| RecordBatch::from(StructArray::from(data))))
+            Some(result.map(|data| {
+                let struct_array = StructArray::from(data);

Review Comment:
   Can you please leave a comment here explaining:
   1. The rationale for this form rather than just converting StructArray to 
RecordBatch
   2. A `SAFETY` comment explaining why the `unsafe` is ok (aka how the 
invariants required for RecordBatch::new_unchecked are satisfied)



##########
arrow-pyarrow/src/lib.rs:
##########
@@ -484,6 +487,100 @@ impl IntoPyArrow for ArrowArrayStreamReader {
     }
 }
 
+/// This is a convenience wrapper around `Vec<RecordBatch>` that tries to 
simplify conversion from

Review Comment:
   I am not familiar enough with how the python interface works to know if this 
is reasonable or not. Perhaps @kylebarron  can help review this part



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