Copilot commented on code in PR #50929:
URL: https://github.com/apache/arrow/pull/50929#discussion_r3903024861
##########
python/pyarrow/array.pxi:
##########
@@ -4967,12 +4992,7 @@ cdef class FixedShapeTensorArray(ExtensionArray):
along the first dimension.
"""
- cdef:
- CFixedShapeTensorArray* ext_array =
<CFixedShapeTensorArray*>(self.ap)
- CResult[shared_ptr[CTensor]] ctensor
- with nogil:
- ctensor = ext_array.ToTensor()
- return pyarrow_wrap_tensor(GetResultValue(ctensor))
+ return Array.to_tensor(self)
Review Comment:
FixedShapeTensorArray defines its own to_tensor() (no allow_nulls
parameter), which now just calls Array.to_tensor(self). This override hides the
new Array.to_tensor(allow_nulls=...) API, so callers can’t pass allow_nulls for
FixedShapeTensorArray.
##########
cpp/src/arrow/c/dlpack.cc:
##########
@@ -130,12 +121,12 @@ DT* ExportBuffer(ExportBufferParams<Vec>&& p) {
template <typename DT>
Result<DT*> ExportArrayImpl(const std::shared_ptr<Array>& arr, bool copy) {
- // Define DLDevice struct and check if array type is supported
- // by the DLPack protocol at the same time. Raise TypeError if not.
- // Supported data types: int, uint, float with no validity buffer.
+ if (arr->null_count() > 0) {
+ return Status::TypeError("Can only use DLPack on arrays with no nulls.");
+ }
ARROW_ASSIGN_OR_RAISE(auto device, ExportDevice(arr));
- // Define the DLDataType struct
+ // Define the DLDataType struct, or fail if the type is not supported.
const auto& type = *arr->type();
ARROW_ASSIGN_OR_RAISE(auto dtype, GetDLDataType(type));
Review Comment:
ExportArrayImpl later dereferences data.buffers[1] in the copy path. For
valid empty arrays it’s legal for the values buffer to be null (ArrayData
length=0 with {nullptr, nullptr}), which would crash when exporting a copied
DLPack capsule.
--
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]
