WillAyd commented on code in PR #637:
URL: https://github.com/apache/arrow-nanoarrow/pull/637#discussion_r1779497703


##########
python/src/nanoarrow/_array.pyx:
##########
@@ -249,6 +294,24 @@ cdef class CArrayView:
 
         return dictionary
 
+    def _iter_bytes(self, int64_t offset, int64_t length):
+        cdef ArrowBufferView item_view
+        for i in range(offset, length):
+            if ArrowArrayViewIsNull(self._ptr, i):
+                yield None
+            else:
+                item_view = ArrowArrayViewGetBytesUnsafe(self._ptr, i)
+                yield PyBytes_FromStringAndSize(item_view.data.as_char, 
item_view.size_bytes)
+
+    def _iter_str(self, int64_t offset, int64_t length):

Review Comment:
   ```suggestion
       def _iter_str(self, int64_t offset, int64_t length) -> str | None:
   ```



##########
python/src/nanoarrow/_array.pyx:
##########
@@ -189,13 +196,48 @@ cdef class CArrayView:
 
     @property
     def n_buffers(self):
+        if _types.is_data_view(self._ptr.storage_type):
+            return 2 + self._ptr.n_variadic_buffers + 1
+
         return self.layout.n_buffers
 
-    def buffer_type(self, int64_t i):
+    def _buffer_info(self, int64_t i):
         if i < 0 or i >= self.n_buffers:
             raise IndexError(f"{i} out of range [0, {self.n_buffers}]")
 
-        buffer_type = self._ptr.layout.buffer_type[i]
+        if (
+            _types.is_data_view(self._ptr.storage_type)
+            and i == (2 + self._ptr.n_variadic_buffers)
+        ):
+            return (
+                NANOARROW_BUFFER_TYPE_DATA,

Review Comment:
   Checking the type of the variadic buffer is actually missing from the C++ 
test; we might want to go back and add this there too



##########
python/src/nanoarrow/_array.pyx:
##########
@@ -249,6 +294,24 @@ cdef class CArrayView:
 
         return dictionary
 
+    def _iter_bytes(self, int64_t offset, int64_t length):

Review Comment:
   ```suggestion
       def _iter_bytes(self, int64_t offset, int64_t length) -> bytes | None:
   ```
   
   There _might_ be some performance benefit from Cython to declaring the 
return type here, especially since almost everything else looks to be typed. 



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