danepitkin commented on code in PR #409:
URL: https://github.com/apache/arrow-nanoarrow/pull/409#discussion_r1550030752
##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -1115,6 +1149,11 @@ cdef class CArray:
"""
self._assert_valid()
+ if self._device_type != ARROW_DEVICE_CPU:
+ raise ValueError(
+ "Can't invoke __arrow_c_aray__ on non-CPU array "
Review Comment:
```suggestion
"Can't invoke __arrow_c_array__ on non-CPU array "
```
##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -2340,7 +2404,53 @@ cdef class CDeviceArray:
@property
def array(self):
- return CArray(self, <uintptr_t>&self._ptr.array, self._schema)
+ # TODO: We loose access to the sync_event here, so we probably need to
+ # synchronize (or propatate it, or somehow prevent data access
downstream)
+ cdef CArray array = CArray(self, <uintptr_t>&self._ptr.array,
self._schema)
+ array._set_device(self._ptr.device_type, self._ptr.device_id)
+ return array
+
+ def view(self):
+ return self.array.view()
+
+ def __arrow_c_array__(self, requested_schema=None):
+ return self.array.__arrow_c_array__(requested_schema=requested_schema)
+
+ def __arrow_c_device_array__(self, requested_schema=None):
+ if requested_schema is not None:
+ raise NotImplementedError("requested_schema")
+
+ # TODO: evaluate whether we need to synchronize here or whether we
should
+ # move device arrays instead of shallow-copying them
+ device_array_capsule = alloc_c_device_array_shallow_copy(self._base,
self._ptr)
+ return self._schema.__arrow_c_schema__(), device_array_capsule
+
+ @staticmethod
+ def _import_from_c_capsule(schema_capsule, device_array_capsule):
+ """
+ Import from a ArrowSchema and ArrowArray PyCapsule tuple.
Review Comment:
```suggestion
Import from an ArrowSchema and ArrowArray PyCapsule tuple.
```
##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -2340,7 +2404,53 @@ cdef class CDeviceArray:
@property
def array(self):
- return CArray(self, <uintptr_t>&self._ptr.array, self._schema)
+ # TODO: We loose access to the sync_event here, so we probably need to
Review Comment:
```suggestion
# TODO: We lose access to the sync_event here, so we probably need to
```
##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -2340,7 +2404,53 @@ cdef class CDeviceArray:
@property
def array(self):
- return CArray(self, <uintptr_t>&self._ptr.array, self._schema)
+ # TODO: We loose access to the sync_event here, so we probably need to
+ # synchronize (or propatate it, or somehow prevent data access
downstream)
Review Comment:
```suggestion
# synchronize (or propagate it, or somehow prevent data access
downstream)
```
##########
python/tests/test_device.py:
##########
@@ -31,12 +30,51 @@ def test_cpu_device():
cpu = device.resolve(1, 0)
assert cpu.device_type == 1
- pa_array = pa.array([1, 2, 3])
- darray = device.c_device_array(pa_array)
+def test_c_device_array():
+ # Unrecognized arguments should be passed to c_array() to generate CPU
array
+ darray = device.c_device_array([1, 2, 3], na.int32())
+
assert darray.device_type == 1
assert darray.device_id == 0
Review Comment:
Out of curiosity, are there enums that can be used here instead of values 1,
0? Or do we need to wait for DLPack support.
##########
python/src/nanoarrow/c_lib.py:
##########
@@ -427,7 +427,7 @@ def c_array_view(obj, schema=None) -> CArrayView:
if isinstance(obj, CArrayView) and schema is None:
return obj
- return CArrayView.from_array(c_array(obj, schema))
+ return c_array(obj, schema).view()
Review Comment:
Very cool!
##########
python/src/nanoarrow/_lib.pyx:
##########
@@ -1038,6 +1064,12 @@ cdef class CArray:
self._base = base
self._ptr = <ArrowArray*>addr
self._schema = schema
+ self._device_type = ARROW_DEVICE_CPU
+ self._device_id = 0
+
+ cdef _set_device(self, ArrowDeviceType device_type, int64_t device_id):
+ self._device_type = device_type
+ self._device_id = device_id
Review Comment:
This function is called frequently after initialization. Is it worth
allowing `__cinit__` to set device type/id?
##########
python/tests/test_device.py:
##########
@@ -31,12 +30,51 @@ def test_cpu_device():
cpu = device.resolve(1, 0)
assert cpu.device_type == 1
- pa_array = pa.array([1, 2, 3])
- darray = device.c_device_array(pa_array)
+def test_c_device_array():
+ # Unrecognized arguments should be passed to c_array() to generate CPU
array
+ darray = device.c_device_array([1, 2, 3], na.int32())
+
assert darray.device_type == 1
assert darray.device_id == 0
- assert darray.array.length == 3
assert "device_type: 1" in repr(darray)
Review Comment:
If there are enums, it would be nice to print the name (e.g. `device_type: 1
(CPU)`)
--
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]
