danepitkin commented on code in PR #550:
URL: https://github.com/apache/arrow-nanoarrow/pull/550#discussion_r1764056640
##########
python/src/nanoarrow/_buffer.pyx:
##########
@@ -146,6 +178,32 @@ cdef object view_to_dlpack(CBufferView view):
Py_INCREF(view)
dlm_tensor.deleter = view_dlpack_deleter
+ # stream has a DLPack + device specific interpretation
+
+ # nanoarrow_device needs a CUstream* (i.e., a CUstream_st**), but dlpack
+ # gives us a CUstream_st*.
+ cdef void* cuda_pstream
+
+ if view._event.device is DEVICE_CPU:
+ if stream is not None and stream != -1:
+ raise ValueError("dlpack stream must be None or -1 for the CPU
device")
+ elif view._event.device.device_type_id == ARROW_DEVICE_CUDA:
+ if stream == 0:
+ raise ValueError("dlpack stream value of 0 is not permitted for
CUDA")
+ elif stream == -1:
+ # Sentinel for "do not synchronize"
+ pass
+ elif stream in (1, 2):
+ # Technically we are mixing the per-thread and legacy default
streams here;
+ # however, the nanoarrow_device API currently has no mechanism to
expose
+ # a pointer these streams specifically.
Review Comment:
```suggestion
# a pointer to these streams specifically.
```
##########
python/src/nanoarrow/_buffer.pyx:
##########
@@ -549,26 +623,103 @@ cdef class CBuffer:
self._view = CBufferView(
self._base, <uintptr_t>self._ptr.data,
self._ptr.size_bytes, self._data_type, self._element_size_bits,
- self._device
+ CSharedSyncEvent(self._device)
)
snprintf(self._view._format, sizeof(self._view._format), "%s",
self._format)
+ def view(self):
+ """Export this buffer as a CBufferView
+
+ Returns a :class:`CBufferView` of this buffer. After calling this
+ method, the original CBuffer will be invalidated and cannot be used.
+ In general, the view of the buffer should be used to consume a buffer
+ (whereas the CBuffer is primarily used to wrap an existing object in
+ a way that it can be used to build a :class:`CArray`).
+ """
+ self._assert_valid()
+ self._assert_buffer_count_zero()
+ cdef ArrowBuffer* new_ptr
+ self._view._base = _utils.alloc_c_buffer(&new_ptr)
+ ArrowBufferMove(self._ptr, new_ptr)
+ self._ptr = NULL
+ return self._view
+
@staticmethod
def empty():
+ """Create an emtpy CBuffer"""
Review Comment:
```suggestion
"""Create an empty CBuffer"""
```
##########
python/tests/test_c_array.py:
##########
@@ -610,3 +622,91 @@ def test_c_array_duration():
d2_duration_in_ms,
d3_duration_in_ms,
]
+
+
+def test_device_array_errors():
+ from nanoarrow.device import DeviceType, resolve
+
+ try:
+ cuda_device = resolve(DeviceType.CUDA, 0)
+ except ValueError:
+ pytest.skip("CUDA device not available")
Review Comment:
It could be nice to put this in a pytest.fixture if you want to make it
reusable e.g.
```
@pytest.fixture
def cuda_device():
from nanoarrow.device import resolve
try:
return resolve(DeviceType.CUDA, 0)
except ValueError:
pytest.skip("CUDA device not available")
def test_device_array_errors(cuda_device):
...
```
This should still skip the tests that require the pytest fixture. Add it to
conftest.py to use it across test files.
--
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]
