jorisvandenbossche commented on code in PR #39980:
URL: https://github.com/apache/arrow/pull/39980#discussion_r1483160913
##########
cpp/src/arrow/c/bridge.cc:
##########
@@ -1977,6 +1978,24 @@ Result<std::shared_ptr<Array>> ImportDeviceArray(struct
ArrowDeviceArray* array,
return ImportDeviceArray(array, *maybe_type, mapper);
}
+Result<std::shared_ptr<MemoryManager>> DefaultDeviceMapper(ArrowDeviceType
device_type,
+ int64_t device_id) {
+ if (device_type != ARROW_DEVICE_CPU) {
+ return Status::NotImplemented("Only importing data on CPU is supported");
+ }
+ return default_cpu_memory_manager();
+}
+
+Result<std::shared_ptr<Array>> ImportDeviceArray(struct ArrowDeviceArray*
array,
+ std::shared_ptr<DataType>
type) {
+ return ImportDeviceArray(array, type, DefaultDeviceMapper);
+}
Review Comment:
Do we want to provide such an API that uses a default DeviceMapper?
With the current APIs here, I assume the idea is that it's the
responsibility of the user (i.e. the library or application using Arrow C++ to
consume data through the C Device interface) to provide the device mapping as
they see fit.
In the case of exposing this in pyarrow, it's pyarrow that is the user of
those APIs and I think pyarrow certainly wants to have a default mapping
provided (not to be specified by the user of pyarrow). In theory I could write
this `DefaultDeviceMapper` function in cython to keep this on the pyarrow side,
but this might also be useful for other users of the C++ APIs?
(I suppose when we add a default in C++, I could also give the existing
signatures a default parameter value for `mapper`, instead of adding those two
additional signatures)
cc @zeroshade @pitrou
--
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]
