Copilot commented on code in PR #50929:
URL: https://github.com/apache/arrow/pull/50929#discussion_r3822639179
##########
cpp/src/arrow/c/dlpack.cc:
##########
@@ -167,6 +158,17 @@ Result<DT*> ExportArrayImpl(const std::shared_ptr<Array>&
arr, bool copy) {
return ExportBuffer<DT>(std::move(params));
}
+template <typename T>
+Result<DLDevice> ExportDeviceImpl(const std::shared_ptr<T>& a) {
+ // ArrayData reports the device of its buffers and children
+ if (a->data()->device_type() == DeviceAllocationType::kCPU) {
+ return {{.device_type = DLDeviceType::kDLCPU, .device_id = 0}};
+ } else {
+ return Status::NotImplemented(
+ "DLPack support is implemented only for buffers on CPU device.");
+ }
Review Comment:
`ExportDeviceImpl` uses C++20 designated initializers when returning
`DLDevice`, which can break builds if the project is compiled as C++17. Prefer
explicit field assignment to keep this code standard-compatible.
##########
cpp/src/arrow/array/array_nested.cc:
##########
@@ -1001,6 +1001,40 @@ Result<std::shared_ptr<Array>>
FixedSizeListArray::Flatten(
return FlattenListArray(*this, memory_pool);
}
+Result<std::shared_ptr<Tensor>> FixedSizeListArray::ToTensor() const {
+ const auto* data = this->data().get();
+ auto type = this->type();
Review Comment:
`FixedSizeListArray::ToTensor()` calls `Tensor::Make(...)`, but this
translation unit doesn't include `arrow/tensor.h` (and only forward-declares
`Tensor` via `array_base.h`). This will fail to compile because `Tensor::Make`
isn't declared here. Add the missing include.
--
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]
