AlenkaF commented on code in PR #50827:
URL: https://github.com/apache/arrow/pull/50827#discussion_r3764816261
##########
cpp/src/arrow/c/dlpack.cc:
##########
@@ -59,52 +65,120 @@ Result<DLDataType> GetDLDataType(const DataType& type) {
}
}
+template <typename DT, typename Vec>
struct ManagerCtx {
- std::shared_ptr<ArrayData> array;
- DLManagedTensor tensor;
+ /// Arrow buffer into of the data
Review Comment:
```suggestion
```
I don't think this info is needed.
##########
python/pyarrow/tensor.pxi:
##########
@@ -310,20 +310,45 @@ strides: {self.strides}"""
A Python integer representing a pointer to a stream. Currently not
supported.
Stream is provided by the consumer to the producer to instruct the
producer
to ensure that operations can safely be performed on the array.
+ max_version : tuple[int, int], optional
+ The maximum DLPack version the consumer supports, as (major,
minor).
+ A capsule of a different version may be returned, so the consumer
must
+ check it. Default is None, exporting the unversioned capsule.
+ dl_device : tuple[enum.Enum, int], optional
+ The device of the exported capsule, in the format returned by
+ :meth:`__dlpack_device__`. Default is None, meaning the device of
the
+ tensor itself. Since only CPU tensors are supported, any other
device
+ raises ``BufferError``.
+ copy : bool, optional
+ If True, the data is always copied. If False, it is never copied
and
+ ``BufferError`` is raised if a copy is required. If None
(default), the
+ data is copied only if needed, which for CPU tensors is never.
Review Comment:
If `copy` is `None` by default, where is this decision made to copy only if
needed? Is it not that `None` somehow gets defined to be `True` or `False` in
the Cython layer?
##########
python/pyarrow/tensor.pxi:
##########
@@ -310,20 +310,45 @@ strides: {self.strides}"""
A Python integer representing a pointer to a stream. Currently not
supported.
Stream is provided by the consumer to the producer to instruct the
producer
to ensure that operations can safely be performed on the array.
+ max_version : tuple[int, int], optional
+ The maximum DLPack version the consumer supports, as (major,
minor).
+ A capsule of a different version may be returned, so the consumer
must
+ check it. Default is None, exporting the unversioned capsule.
+ dl_device : tuple[enum.Enum, int], optional
+ The device of the exported capsule, in the format returned by
+ :meth:`__dlpack_device__`. Default is None, meaning the device of
the
+ tensor itself. Since only CPU tensors are supported, any other
device
+ raises ``BufferError``.
+ copy : bool, optional
+ If True, the data is always copied. If False, it is never copied
and
+ ``BufferError`` is raised if a copy is required. If None
(default), the
+ data is copied only if needed, which for CPU tensors is never.
+ A copy is reported to the consumer with
``DLPACK_FLAG_BITMASK_IS_COPIED``.
Returns
-------
capsule : PyCapsule
- A DLPack capsule for the tensor, pointing to a DLManagedTensor.
- """
- if stream is None:
- dlm_tensor = GetResultValue(ExportTensorToDLPack(self.sp_tensor))
-
- return PyCapsule_New(dlm_tensor, 'dltensor',
dlpack_pycapsule_deleter)
- else:
- raise NotImplementedError(
- "Only stream=None is supported."
- )
+ A DLPack capsule for the tensor, pointing to a
DLManagedTensorVersioned,
+ or to a DLManagedTensor if ``max_version`` is below (1, 0).
+ """
+ if stream is not None:
+ raise NotImplementedError("Only stream=None is supported.")
+ if dl_device is not None:
+ device = GetResultValue(ExportDevice(self.sp_tensor))
+ if dl_device != (device.device_type, device.device_id):
+ raise BufferError(
+ f"Cannot export to device {dl_device}, "
+ f"tensor is on {(device.device_type, device.device_id)}."
+ )
+ if max_version is None or max_version < (1, 0):
Review Comment:
I think we should raise in cases where `copy=True` is used with a
legacy/unversioned capsule. Also adding that to the test suite as now it seems
it is presumed `copy=True` will not be used with unversioned option - but one
could still do that in which case copy keyword would silently get ignored.
--
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]
