From 24b1f68312492270aa6b7dff2ddecf78f9474172 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 20 Mar 2025 21:09:11 +0000 Subject: [PATCH 01/41] First pass of using the new APIs --- python/pylibcudf/pylibcudf/interop.pyx | 43 +++++++++++++------ .../pylibcudf/pylibcudf/libcudf/interop.pxd | 18 +++++++- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 7a102cf0c881..c35b9eba4660 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -1,4 +1,6 @@ -# Copyright (c) 2023-2024, NVIDIA CORPORATION. +# Copyright (c) 2023-2025, NVIDIA CORPORATION. + +from cython.operator cimport dereference from cpython.pycapsule cimport ( PyCapsule_GetPointer, @@ -7,7 +9,7 @@ from cpython.pycapsule cimport ( PyCapsule_SetName, ) from libc.stdlib cimport free -from libcpp.memory cimport unique_ptr +from libcpp.memory cimport make_unique, unique_ptr from libcpp.utility cimport move from libcpp.vector cimport vector @@ -16,15 +18,14 @@ from functools import singledispatch from pyarrow import lib as pa -from pylibcudf.libcudf.column.column cimport column from pylibcudf.libcudf.interop cimport ( ArrowArray, ArrowArrayStream, ArrowSchema, DLManagedTensor, + arrow_table, + arrow_column, column_metadata, - from_arrow_column as cpp_from_arrow_column, - from_arrow_stream as cpp_from_arrow_stream, from_dlpack as cpp_from_dlpack, to_arrow_host_raw, to_arrow_schema_raw, @@ -135,6 +136,14 @@ def _from_arrow_datatype(pyarrow_object): raise TypeError(f"Unable to convert {pyarrow_object} to cudf datatype") +cdef class _ArrowColumnHolder: + cdef unique_ptr[arrow_column] col + + +cdef class _ArrowTableHolder: + cdef unique_ptr[arrow_table] tbl + + @from_arrow.register(pa.Table) def _from_arrow_table(pyarrow_object, *, DataType data_type=None): if data_type is not None: @@ -144,12 +153,18 @@ def _from_arrow_table(pyarrow_object, *, DataType data_type=None): PyCapsule_GetPointer(stream, "arrow_array_stream") ) - cdef unique_ptr[table] c_result + cdef _ArrowTableHolder result = _ArrowTableHolder() + cdef unique_ptr[arrow_table] c_result + with nogil: - # The libcudf function here will release the stream. - c_result = cpp_from_arrow_stream(c_stream) + c_result = make_unique[arrow_table](move(dereference(c_stream))) + result.tbl.swap(c_result) + + # The capsule destructor should release automatically for us, but we choose to do it + # explicitly here for clarity. + c_stream.release(c_stream) - return Table.from_libcudf(move(c_result)) + return Table.from_table_view_of_arbitrary(result.tbl.get().view(), result) @from_arrow.register(pa.Scalar) @@ -180,16 +195,20 @@ def _from_arrow_column(pyarrow_object, *, DataType data_type=None): PyCapsule_GetPointer(array, "arrow_array") ) - cdef unique_ptr[column] c_result + cdef _ArrowColumnHolder result = _ArrowColumnHolder() + cdef unique_ptr[arrow_column] c_result with nogil: - c_result = cpp_from_arrow_column(c_schema, c_array) + c_result = make_unique[arrow_column]( + move(dereference(c_schema)), move(dereference(c_array)) + ) + result.col.swap(c_result) # The capsule destructors should release automatically for us, but we # choose to do it explicitly here for clarity. c_schema.release(c_schema) c_array.release(c_array) - return Column.from_libcudf(move(c_result)) + return Column.from_column_view_of_arbitrary(result.col.get().view(), result) @singledispatch diff --git a/python/pylibcudf/pylibcudf/libcudf/interop.pxd b/python/pylibcudf/pylibcudf/libcudf/interop.pxd index 8953357a087f..1d1cf34838ad 100644 --- a/python/pylibcudf/pylibcudf/libcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/libcudf/interop.pxd @@ -1,4 +1,4 @@ -# Copyright (c) 2020-2024, NVIDIA CORPORATION. +# Copyright (c) 2020-2025, NVIDIA CORPORATION. from libcpp.memory cimport shared_ptr, unique_ptr from libcpp.string cimport string from libcpp.vector cimport vector @@ -55,6 +55,22 @@ cdef extern from "cudf/interop.hpp" namespace "cudf" \ ) except +libcudf_exception_handler +cdef extern from "cudf/interop.hpp" namespace "cudf::interop" \ + nogil: + cdef cppclass arrow_column: + arrow_column( + ArrowSchema* schema, + ArrowArray* array + ) except +libcudf_exception_handler + column_view view() except +libcudf_exception_handler + + cdef cppclass arrow_table: + arrow_table( + ArrowArrayStream&& stream, + ) except +libcudf_exception_handler + table_view view() except +libcudf_exception_handler + + cdef extern from *: # Rather than exporting the underlying functions directly to Cython, we expose # these wrappers that handle the release to avoid needing to teach Cython how From a09a31d8abefff6ed8e51969b14ca8fe34c41410 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 20 Mar 2025 22:12:40 +0000 Subject: [PATCH 02/41] Fix some C++ API docs --- cpp/include/cudf/interop.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/cudf/interop.hpp b/cpp/include/cudf/interop.hpp index e60038419ff1..7f01d3345f4e 100644 --- a/cpp/include/cudf/interop.hpp +++ b/cpp/include/cudf/interop.hpp @@ -314,7 +314,7 @@ class arrow_column { * * @param stream CUDA stream used for device memory operations and kernel launches * @param mr Device memory resource used for any allocations during conversion - * @return unique_column_view_t containing a view of the column data + * @return A view of the column data */ [[nodiscard]] column_view view() const; @@ -434,7 +434,7 @@ class arrow_table { * * @param stream CUDA stream used for device memory operations and kernel launches * @param mr Device memory resource used for any allocations during conversion - * @return unique_table_view_t containing a view of the table data + * @return A view of the table data */ [[nodiscard]] table_view view() const; From 5d2884c3136d1eee206b73efeaa00f669a27712d Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 20 Mar 2025 22:12:55 +0000 Subject: [PATCH 03/41] Remove now unused APIs --- python/pylibcudf/pylibcudf/libcudf/interop.pxd | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/python/pylibcudf/pylibcudf/libcudf/interop.pxd b/python/pylibcudf/pylibcudf/libcudf/interop.pxd index 1d1cf34838ad..2ae76d6ada32 100644 --- a/python/pylibcudf/pylibcudf/libcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/libcudf/interop.pxd @@ -46,21 +46,13 @@ cdef extern from "cudf/interop.hpp" namespace "cudf" \ string name vector[column_metadata] children_meta - cdef unique_ptr[table] from_arrow_stream( - ArrowArrayStream* input - ) except +libcudf_exception_handler - cdef unique_ptr[column] from_arrow_column( - const ArrowSchema* schema, - const ArrowArray* input - ) except +libcudf_exception_handler - cdef extern from "cudf/interop.hpp" namespace "cudf::interop" \ nogil: cdef cppclass arrow_column: arrow_column( - ArrowSchema* schema, - ArrowArray* array + ArrowSchema&& schema, + ArrowArray&& array ) except +libcudf_exception_handler column_view view() except +libcudf_exception_handler From b4760acf87a217f448843028f94e215aa6052347 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 21 Mar 2025 00:04:39 +0000 Subject: [PATCH 04/41] Don't call release manually anymore --- python/pylibcudf/pylibcudf/interop.pyx | 9 --------- 1 file changed, 9 deletions(-) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index c35b9eba4660..be721ad1659a 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -160,10 +160,6 @@ def _from_arrow_table(pyarrow_object, *, DataType data_type=None): c_result = make_unique[arrow_table](move(dereference(c_stream))) result.tbl.swap(c_result) - # The capsule destructor should release automatically for us, but we choose to do it - # explicitly here for clarity. - c_stream.release(c_stream) - return Table.from_table_view_of_arbitrary(result.tbl.get().view(), result) @@ -203,11 +199,6 @@ def _from_arrow_column(pyarrow_object, *, DataType data_type=None): ) result.col.swap(c_result) - # The capsule destructors should release automatically for us, but we - # choose to do it explicitly here for clarity. - c_schema.release(c_schema) - c_array.release(c_array) - return Column.from_column_view_of_arbitrary(result.col.get().view(), result) From 72ef25d53541d17ba12462062fe00559b9f9c30a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 21 Mar 2025 00:16:57 +0000 Subject: [PATCH 05/41] Move table to arrow interop functions to Table class via capsule --- python/pylibcudf/pylibcudf/CMakeLists.txt | 1 + python/pylibcudf/pylibcudf/interop.pyx | 93 +----------------- python/pylibcudf/pylibcudf/table.pyx | 109 ++++++++++++++++++++++ 3 files changed, 111 insertions(+), 92 deletions(-) diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index 147060f87473..f85e65733f3a 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -66,6 +66,7 @@ target_include_directories(pylibcudf_interop PUBLIC "$PyCapsule_GetPointer( - schema_capsule, 'arrow_schema' - ) - if schema.release != NULL: - schema.release(schema) - - free(schema) - - -cdef void _release_array(object array_capsule) noexcept: - """Release the ArrowArray object stored in a PyCapsule.""" - cdef ArrowArray* array = PyCapsule_GetPointer( - array_capsule, 'arrow_array' - ) - if array.release != NULL: - array.release(array) - - free(array) - - -def _maybe_create_nested_column_metadata(Column col): - return ColumnMetadata( - children_meta=[ - _maybe_create_nested_column_metadata(child) for child in col.children() - ] - ) - - -def _table_to_schema(Table tbl, metadata): - if metadata is None: - metadata = [_maybe_create_nested_column_metadata(col) for col in tbl.columns()] - else: - metadata = [ColumnMetadata(m) if isinstance(m, str) else m for m in metadata] - - cdef vector[column_metadata] c_metadata - c_metadata.reserve(len(metadata)) - for meta in metadata: - c_metadata.push_back(_metadata_to_libcudf(meta)) - - cdef ArrowSchema* raw_schema_ptr - with nogil: - raw_schema_ptr = to_arrow_schema_raw(tbl.view(), c_metadata) - - return PyCapsule_New(raw_schema_ptr, 'arrow_schema', _release_schema) - - -def _table_to_host_array(Table tbl): - cdef ArrowArray* raw_host_array_ptr - with nogil: - raw_host_array_ptr = to_arrow_host_raw(tbl.view()) - - return PyCapsule_New(raw_host_array_ptr, "arrow_array", _release_array) - - -class _TableWithArrowMetadata: - def __init__(self, tbl, metadata=None): - self.tbl = tbl - self.metadata = metadata - - def __arrow_c_array__(self, requested_schema=None): - return _table_to_schema(self.tbl, self.metadata), _table_to_host_array(self.tbl) - - -# TODO: In the long run we should get rid of the `to_arrow` functions in favor of using -# the protocols directly via `pa.table(cudf_object, schema=...)` directly. We can do the -# same for columns. We cannot do this for scalars since there is no corresponding -# protocol. Since this will require broader changes throughout the codebase, the current -# approach is to leverage the protocol internally but to continue exposing `to_arrow`. @to_arrow.register(Table) def _to_arrow_table(cudf_object, metadata=None): - test_table = _TableWithArrowMetadata(cudf_object, metadata) - return pa.table(test_table) + return pa.table(cudf_object) @to_arrow.register(Column) diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 9449aee38382..8f7f9c03dc15 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -1,17 +1,73 @@ # Copyright (c) 2023-2025, NVIDIA CORPORATION. from cython.operator cimport dereference + +from cpython.pycapsule cimport ( + PyCapsule_GetPointer, + PyCapsule_New, +) + +from libc.stdlib cimport free from libcpp.memory cimport unique_ptr from libcpp.utility cimport move from libcpp.vector cimport vector + from pylibcudf.libcudf.column.column cimport column from pylibcudf.libcudf.column.column_view cimport column_view from pylibcudf.libcudf.table.table cimport table +from pylibcudf.libcudf.interop cimport ( + ArrowArray, + ArrowSchema, + column_metadata, + to_arrow_host_raw, + to_arrow_schema_raw, +) + from .column cimport Column __all__ = ["Table"] + +# TODO: Add a strong type here on the ColumnMetadata input +cdef column_metadata _metadata_to_libcudf(metadata): + """Convert a ColumnMetadata object to C++ column_metadata. + + Since this class is mutable and cheap, it is easier to create the C++ + object on the fly rather than have it directly backing the storage for + the Cython class. Additionally, this structure restricts the dependency + on C++ types to just within this module, allowing us to make the module a + pure Python module (from an import sense, i.e. no pxd declarations). + """ + cdef column_metadata c_metadata + c_metadata.name = metadata.name.encode() + for child_meta in metadata.children_meta: + c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) + return c_metadata + + +cdef void _release_schema(object schema_capsule) noexcept: + """Release the ArrowSchema object stored in a PyCapsule.""" + cdef ArrowSchema* schema = PyCapsule_GetPointer( + schema_capsule, 'arrow_schema' + ) + if schema.release != NULL: + schema.release(schema) + + free(schema) + + +cdef void _release_array(object array_capsule) noexcept: + """Release the ArrowArray object stored in a PyCapsule.""" + cdef ArrowArray* array = PyCapsule_GetPointer( + array_capsule, 'arrow_array' + ) + if array.release != NULL: + array.release(array) + + free(array) + + cdef class Table: """A list of columns of the same size. @@ -114,3 +170,56 @@ cdef class Table: cpdef list columns(self): """The columns in this table.""" return self._columns + + @staticmethod + def _create_nested_column_metadata(Column col): + # TODO: We'll need to reshuffle where things are defined to avoid circular + # imports. For now, we'll just import this inline. We should be able to avoid + # circularity altogether by simply + from pylibcudf.interop import ColumnMetadata + return ColumnMetadata( + children_meta=[ + Table._create_nested_column_metadata(child) for child in col.children() + ] + ) + + def _to_schema(self, metadata=None): + """Create an Arrow schema from this table.""" + # TODO: We'll need to reshuffle where things are defined to avoid circular + # imports. For now, we'll just import this inline. We should be able to avoid + # circularity altogether by simply + from pylibcudf.interop import ColumnMetadata + if metadata is None: + metadata = [ + Table._create_nested_column_metadata(col) for col in self.columns() + ] + else: + metadata = [ + ColumnMetadata(m) if isinstance(m, str) else m for m in metadata + ] + + cdef vector[column_metadata] c_metadata + c_metadata.reserve(len(metadata)) + for meta in metadata: + c_metadata.push_back(_metadata_to_libcudf(meta)) + + cdef ArrowSchema* raw_schema_ptr + with nogil: + raw_schema_ptr = to_arrow_schema_raw(self.view(), c_metadata) + + return PyCapsule_New(raw_schema_ptr, 'arrow_schema', _release_schema) + + def _to_host_array(self): + cdef ArrowArray* raw_host_array_ptr + with nogil: + raw_host_array_ptr = to_arrow_host_raw(self.view()) + + return PyCapsule_New(raw_host_array_ptr, "arrow_array", _release_array) + + def __arrow_c_array__(self, requested_schema=None): + if requested_schema is not None: + raise ValueError("pylibcudf.Table does not support alternative schema") + + # For the host array protocol the capsules own the data. + ret = self._to_schema(), self._to_host_array() + return ret From 4bafc1f117fea8bd7b13a97dca161f9d59775fe0 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 21 Mar 2025 23:44:59 +0000 Subject: [PATCH 06/41] Move from_arrow table interop to Table constructor overload --- python/pylibcudf/pylibcudf/interop.pyx | 20 +-------- python/pylibcudf/pylibcudf/table.pyx | 57 ++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 24d90f96a482..b6230d4233cc 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -18,10 +18,8 @@ from pyarrow import lib as pa from pylibcudf.libcudf.interop cimport ( ArrowArray, - ArrowArrayStream, ArrowSchema, DLManagedTensor, - arrow_table, arrow_column, from_dlpack as cpp_from_dlpack, to_dlpack as cpp_to_dlpack, @@ -120,27 +118,11 @@ cdef class _ArrowColumnHolder: cdef unique_ptr[arrow_column] col -cdef class _ArrowTableHolder: - cdef unique_ptr[arrow_table] tbl - - @from_arrow.register(pa.Table) def _from_arrow_table(pyarrow_object, *, DataType data_type=None): if data_type is not None: raise ValueError("data_type may not be passed for tables") - stream = pyarrow_object.__arrow_c_stream__() - cdef ArrowArrayStream* c_stream = ( - PyCapsule_GetPointer(stream, "arrow_array_stream") - ) - - cdef _ArrowTableHolder result = _ArrowTableHolder() - cdef unique_ptr[arrow_table] c_result - - with nogil: - c_result = make_unique[arrow_table](move(dereference(c_stream))) - result.tbl.swap(c_result) - - return Table.from_table_view_of_arbitrary(result.tbl.get().view(), result) + return Table(pyarrow_object) @from_arrow.register(pa.Scalar) diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 8f7f9c03dc15..9c6a38f00fe6 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -8,17 +8,21 @@ from cpython.pycapsule cimport ( ) from libc.stdlib cimport free -from libcpp.memory cimport unique_ptr +from libcpp.memory cimport unique_ptr, make_unique from libcpp.utility cimport move from libcpp.vector cimport vector +from functools import singledispatchmethod + from pylibcudf.libcudf.column.column cimport column from pylibcudf.libcudf.column.column_view cimport column_view from pylibcudf.libcudf.table.table cimport table from pylibcudf.libcudf.interop cimport ( ArrowArray, + ArrowArrayStream, ArrowSchema, + arrow_table, column_metadata, to_arrow_host_raw, to_arrow_schema_raw, @@ -68,6 +72,24 @@ cdef void _release_array(object array_capsule) noexcept: free(array) +class _ArrowLikeMeta(type): + # Unfortunately we cannot separate stream and array via singledispatch because the + # dispatch will often be ambiguous when objects expose both protocols. + def __subclasscheck__(cls, other): + return ( + hasattr(other, "__arrow_c_stream__") + or hasattr(other, "__arrow_c_array__") + ) + + +class _ArrowLike(metaclass=_ArrowLikeMeta): + pass + + +cdef class _ArrowTableHolder: + cdef unique_ptr[arrow_table] tbl + + cdef class Table: """A list of columns of the same size. @@ -76,12 +98,41 @@ cdef class Table: columns : list The columns in this table. """ - def __init__(self, list columns): + def __init__(self, obj): + self._init(obj) + + __hash__ = None + + @singledispatchmethod + def _init(self, obj): + raise ValueError("Table should be constructed with a list of columns") + + @_init.register(list) + def _(self, list columns): if not all(isinstance(c, Column) for c in columns): raise ValueError("All columns must be pylibcudf Column objects") self._columns = columns - __hash__ = None + @_init.register(_ArrowLike) + def _(self, arrow_like): + cdef ArrowArrayStream* c_stream + cdef _ArrowTableHolder result + cdef unique_ptr[arrow_table] c_result + if hasattr(arrow_like, "__arrow_c_stream__"): + stream = arrow_like.__arrow_c_stream__() + c_stream = ( + PyCapsule_GetPointer(stream, "arrow_array_stream") + ) + + result = _ArrowTableHolder() + with nogil: + c_result = make_unique[arrow_table](move(dereference(c_stream))) + result.tbl.swap(c_result) + + tmp = Table.from_table_view_of_arbitrary(result.tbl.get().view(), result) + self._columns = tmp.columns() + elif hasattr(arrow_like, "__arrow_c_array__"): + raise NotImplementedError("arrays not yet supported") cdef table_view view(self) nogil: """Generate a libcudf table_view to pass to libcudf algorithms. From 17ce4235ddf5c1215afa845ad74ddbe92555de56 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 22 Mar 2025 00:34:02 +0000 Subject: [PATCH 07/41] Move column from_arrow logic to constructor --- python/pylibcudf/pylibcudf/CMakeLists.txt | 1 + python/pylibcudf/pylibcudf/column.pyx | 67 ++++++++++++++++++++++- python/pylibcudf/pylibcudf/interop.pyx | 29 +--------- python/pylibcudf/pylibcudf/table.pyx | 2 +- 4 files changed, 69 insertions(+), 30 deletions(-) diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index f85e65733f3a..9aea24bb410f 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -67,6 +67,7 @@ include(${rapids-cmake-dir}/export/find_package_root.cmake) include(../../../cpp/cmake/thirdparty/get_nanoarrow.cmake) target_link_libraries(pylibcudf_interop PUBLIC nanoarrow) target_link_libraries(pylibcudf_table PUBLIC nanoarrow) +target_link_libraries(pylibcudf_column PUBLIC nanoarrow) add_subdirectory(libcudf) add_subdirectory(strings) diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index d5e7327c12ea..4221e0b0fab8 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -1,8 +1,14 @@ # Copyright (c) 2023-2025, NVIDIA CORPORATION. from cython.operator cimport dereference + +from cpython.pycapsule cimport ( + PyCapsule_GetPointer, +) + from libcpp.memory cimport make_unique, unique_ptr from libcpp.utility cimport move + from pylibcudf.libcudf.column.column cimport column, column_contents from pylibcudf.libcudf.column.column_factories cimport make_column_from_scalar from pylibcudf.libcudf.scalar.scalar cimport scalar @@ -10,6 +16,12 @@ from pylibcudf.libcudf.types cimport size_type from rmm.pylibrmm.device_buffer cimport DeviceBuffer +from pylibcudf.libcudf.interop cimport ( + ArrowArray, + ArrowSchema, + arrow_column, +) + from .gpumemoryview cimport gpumemoryview from .scalar cimport Scalar from .types cimport DataType, size_of, type_id @@ -19,6 +31,20 @@ import functools __all__ = ["Column", "ListColumnView", "is_c_contiguous"] + +class _ArrowLikeMeta(type): + def __subclasscheck__(cls, other): + return hasattr(other, "__arrow_c_array__") + + +class _ArrowLike(metaclass=_ArrowLikeMeta): + pass + + +cdef class _ArrowColumnHolder: + cdef unique_ptr[arrow_column] col + + cdef class Column: """A container of nullable device data as a column of elements. @@ -46,7 +72,17 @@ cdef class Column: children : list The children of this column if it is a compound column type. """ - def __init__( + def __init__(self, obj, *args, **kwargs): + self._init(obj, *args, **kwargs) + + __hash__ = None + + @functools.singledispatchmethod + def _init(self, obj, *args, **kwargs): + raise ValueError(f"Invalid input type {type(obj)}") + + @_init.register(DataType) + def _( self, DataType data_type not None, size_type size, gpumemoryview data, gpumemoryview mask, size_type null_count, size_type offset, list children @@ -62,7 +98,34 @@ cdef class Column: self._children = children self._num_children = len(children) - __hash__ = None + @_init.register(_ArrowLike) + def _(self, arrow_like): + schema, array = arrow_like.__arrow_c_array__() + cdef ArrowSchema* c_schema = ( + PyCapsule_GetPointer(schema, "arrow_schema") + ) + cdef ArrowArray* c_array = ( + PyCapsule_GetPointer(array, "arrow_array") + ) + + cdef _ArrowColumnHolder result = _ArrowColumnHolder() + cdef unique_ptr[arrow_column] c_result + with nogil: + c_result = make_unique[arrow_column]( + move(dereference(c_schema)), move(dereference(c_array)) + ) + result.col.swap(c_result) + + tmp = Column.from_column_view_of_arbitrary(result.col.get().view(), result) + self._init( + tmp.type(), + tmp.size(), + tmp.data(), + tmp.null_mask(), + tmp.null_count(), + tmp.offset(), + tmp.children(), + ) cdef column_view view(self) nogil: """Generate a libcudf column_view to pass to libcudf algorithms. diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index b6230d4233cc..4eac892e3b24 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -1,14 +1,12 @@ # Copyright (c) 2023-2025, NVIDIA CORPORATION. -from cython.operator cimport dereference - from cpython.pycapsule cimport ( PyCapsule_GetPointer, PyCapsule_IsValid, PyCapsule_New, PyCapsule_SetName, ) -from libcpp.memory cimport make_unique, unique_ptr +from libcpp.memory cimport unique_ptr from libcpp.utility cimport move from dataclasses import dataclass, field @@ -17,10 +15,7 @@ from functools import singledispatch from pyarrow import lib as pa from pylibcudf.libcudf.interop cimport ( - ArrowArray, - ArrowSchema, DLManagedTensor, - arrow_column, from_dlpack as cpp_from_dlpack, to_dlpack as cpp_to_dlpack, ) @@ -114,10 +109,6 @@ def _from_arrow_datatype(pyarrow_object): raise TypeError(f"Unable to convert {pyarrow_object} to cudf datatype") -cdef class _ArrowColumnHolder: - cdef unique_ptr[arrow_column] col - - @from_arrow.register(pa.Table) def _from_arrow_table(pyarrow_object, *, DataType data_type=None): if data_type is not None: @@ -145,23 +136,7 @@ def _from_arrow_column(pyarrow_object, *, DataType data_type=None): if data_type is not None: raise ValueError("data_type may not be passed for arrays") - schema, array = pyarrow_object.__arrow_c_array__() - cdef ArrowSchema* c_schema = ( - PyCapsule_GetPointer(schema, "arrow_schema") - ) - cdef ArrowArray* c_array = ( - PyCapsule_GetPointer(array, "arrow_array") - ) - - cdef _ArrowColumnHolder result = _ArrowColumnHolder() - cdef unique_ptr[arrow_column] c_result - with nogil: - c_result = make_unique[arrow_column]( - move(dereference(c_schema)), move(dereference(c_array)) - ) - result.col.swap(c_result) - - return Column.from_column_view_of_arbitrary(result.col.get().view(), result) + return Column(pyarrow_object) @singledispatch diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 9c6a38f00fe6..08c885cea1e4 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -105,7 +105,7 @@ cdef class Table: @singledispatchmethod def _init(self, obj): - raise ValueError("Table should be constructed with a list of columns") + raise ValueError(f"Invalid input type {type(obj)}") @_init.register(list) def _(self, list columns): From 2c65d721ef5ecf482fa1a033e41ac15b20712641 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 22 Mar 2025 01:29:11 +0000 Subject: [PATCH 08/41] Add missing support for empty type --- cpp/src/interop/to_arrow_device.cu | 40 +++++++++++++++++------------ cpp/src/interop/to_arrow_schema.cpp | 8 ++++-- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index ececbc8ebdb6..f7d784f46541 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -91,6 +91,21 @@ int set_buffer(std::unique_ptr device_buf, int64_t i, ArrowArray* out) return NANOARROW_OK; } +int set_null_mask(column::contents& contents, ArrowArray* out) +{ + if (contents.null_mask) { + NANOARROW_RETURN_NOT_OK(set_buffer(std::move(contents.null_mask), validity_buffer_idx, out)); + } + return NANOARROW_OK; +} + +int set_contents(column::contents& contents, ArrowArray* out) +{ + NANOARROW_RETURN_NOT_OK(set_null_mask(contents, out)); + NANOARROW_RETURN_NOT_OK(set_buffer(std::move(contents.data), fixed_width_data_buffer_idx, out)); + return NANOARROW_OK; +} + struct dispatch_to_arrow_device { template () and not is_fixed_point())> @@ -117,21 +132,6 @@ struct dispatch_to_arrow_device { ArrowArrayMove(tmp.get(), out); return NANOARROW_OK; } - - int set_null_mask(column::contents& contents, ArrowArray* out) - { - if (contents.null_mask) { - NANOARROW_RETURN_NOT_OK(set_buffer(std::move(contents.null_mask), validity_buffer_idx, out)); - } - return NANOARROW_OK; - } - - int set_contents(column::contents& contents, ArrowArray* out) - { - NANOARROW_RETURN_NOT_OK(set_null_mask(contents, out)); - NANOARROW_RETURN_NOT_OK(set_buffer(std::move(contents.data), fixed_width_data_buffer_idx, out)); - return NANOARROW_OK; - } }; template <> @@ -548,8 +548,14 @@ unique_device_array_t to_arrow_device(cudf::column&& col, { nanoarrow::UniqueArray tmp; - NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( - col.type(), detail::dispatch_to_arrow_device{}, std::move(col), stream, mr, tmp.get())); + if (col.type().id() == cudf::type_id::EMPTY) { + NANOARROW_THROW_NOT_OK(initialize_array(tmp.get(), NANOARROW_TYPE_NA, col)); + auto contents = col.release(); + NANOARROW_THROW_NOT_OK(set_contents(contents, tmp.get())); + } else { + NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( + col.type(), detail::dispatch_to_arrow_device{}, std::move(col), stream, mr, tmp.get())); + } return create_device_array(std::move(tmp), stream); } diff --git a/cpp/src/interop/to_arrow_schema.cpp b/cpp/src/interop/to_arrow_schema.cpp index aabba447ee22..c375bdd0c4c3 100644 --- a/cpp/src/interop/to_arrow_schema.cpp +++ b/cpp/src/interop/to_arrow_schema.cpp @@ -219,8 +219,12 @@ unique_schema_t to_arrow_schema(cudf::table_view const& input, NANOARROW_THROW_NOT_OK(ArrowSchemaSetName(child, metadata[i].name.c_str())); child->flags = col.has_nulls() ? ARROW_FLAG_NULLABLE : 0; - NANOARROW_THROW_NOT_OK( - cudf::type_dispatcher(col.type(), detail::dispatch_to_arrow_type{}, col, metadata[i], child)); + if (col.type().id() == cudf::type_id::EMPTY) { + ArrowSchemaSetType(child, NANOARROW_TYPE_NA); + } else { + NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( + col.type(), detail::dispatch_to_arrow_type{}, col, metadata[i], child)); + } } unique_schema_t out(new ArrowSchema, [](ArrowSchema* schema) { From 94a2c020b6bf6d3b9345a2aab6e2e01ac0d2265e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 22 Mar 2025 17:11:52 +0000 Subject: [PATCH 09/41] Properly handle length 0 list columns when importing --- cpp/src/interop/from_arrow_device.cu | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cpp/src/interop/from_arrow_device.cu b/cpp/src/interop/from_arrow_device.cu index 836da2987e28..6d634ade3b0a 100644 --- a/cpp/src/interop/from_arrow_device.cu +++ b/cpp/src/interop/from_arrow_device.cu @@ -284,7 +284,7 @@ dispatch_tuple_t dispatch_from_arrow_device::operator()( size_type const offset = input->offset; size_type const null_count = input->null_count; auto offsets_view = column_view{data_type(type_id::INT32), - offset + num_rows + 1, + num_rows == 0 ? 0 : offset + num_rows + 1, input->buffers[fixed_width_data_buffer_idx], nullptr, 0, @@ -300,8 +300,9 @@ dispatch_tuple_t dispatch_from_arrow_device::operator()( // in the scenario where we were sliced and there are more elements in the child_view // than can be referenced by the sliced offsets, we need to slice the child_view // so that when `get_sliced_child` is called, we still produce the right result - auto max_child_offset = cudf::detail::get_value(offsets_view, offset + num_rows, stream); - child_view = cudf::slice(child_view, {0, max_child_offset}, stream).front(); + auto max_child_offset = + num_rows == 0 ? 0 : cudf::detail::get_value(offsets_view, offset + num_rows, stream); + child_view = cudf::slice(child_view, {0, max_child_offset}, stream).front(); return std::make_tuple( {type, From c5cd26b366e3b1a9c82a25e9555ec1751a2ff243 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 22 Mar 2025 17:12:19 +0000 Subject: [PATCH 10/41] Support modifying struct field names when exporting via to_arrow --- python/pylibcudf/pylibcudf/interop.pyx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 4eac892e3b24..6c34799f57a7 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -198,9 +198,21 @@ def _to_arrow_datatype(cudf_object, **kwargs): ) +class _TableWithArrowMetadata: + def __init__(self, tbl, metadata=None): + self.tbl = tbl + self.metadata = metadata + + def __arrow_c_array__(self, requested_schema=None): + return self.tbl._to_schema(self.metadata), self.tbl._to_host_array() + + @to_arrow.register(Table) def _to_arrow_table(cudf_object, metadata=None): - return pa.table(cudf_object) + # TODO: See if we can stop supporting configuration of struct field names when + # exporting to arrow data. That would allow us to get rid of the + # _TableWithArrowMetadata struct and just use the underlying Table directly. + return pa.table(_TableWithArrowMetadata(cudf_object, metadata)) @to_arrow.register(Column) From 219a506858f4f653c1682276f1566ce28d73222d Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 22 Mar 2025 17:51:56 +0000 Subject: [PATCH 11/41] One more fix for empty type --- cpp/src/interop/to_arrow_device.cu | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index f7d784f46541..277298450293 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -535,8 +535,14 @@ unique_device_array_t to_arrow_device(cudf::table&& table, for (size_t i = 0; i < cols.size(); ++i) { auto child = tmp->children[i]; auto col = cols[i].get(); - NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( - col->type(), detail::dispatch_to_arrow_device{}, std::move(*col), stream, mr, child)); + if (col->type().id() == cudf::type_id::EMPTY) { + NANOARROW_THROW_NOT_OK(initialize_array(child, NANOARROW_TYPE_NA, col->view())); + auto contents = col->release(); + NANOARROW_THROW_NOT_OK(set_contents(contents, child)); + } else { + NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( + col->type(), detail::dispatch_to_arrow_device{}, std::move(*col), stream, mr, child)); + } } return create_device_array(std::move(tmp), stream); From 6edcf132c2267a43483c2d90045f320f8ed681c4 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 22 Mar 2025 18:15:13 +0000 Subject: [PATCH 12/41] Change null_count API to accept a gpumemoryview instead of a pointer and use that to fix invalid test --- python/pylibcudf/pylibcudf/null_mask.pxd | 4 +++- python/pylibcudf/pylibcudf/null_mask.pyi | 3 ++- python/pylibcudf/pylibcudf/null_mask.pyx | 5 +++-- .../pylibcudf/pylibcudf/tests/test_null_mask.py | 17 ++++++++++++----- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/python/pylibcudf/pylibcudf/null_mask.pxd b/python/pylibcudf/pylibcudf/null_mask.pxd index 779a5aed306a..bd6969cc415f 100644 --- a/python/pylibcudf/pylibcudf/null_mask.pxd +++ b/python/pylibcudf/pylibcudf/null_mask.pxd @@ -2,6 +2,8 @@ from pylibcudf.libcudf.types cimport mask_state, size_type +from pylibcudf.gpumemoryview cimport gpumemoryview + from rmm.pylibrmm.device_buffer cimport DeviceBuffer from .column cimport Column @@ -17,4 +19,4 @@ cpdef tuple bitmask_and(list columns) cpdef tuple bitmask_or(list columns) -cpdef size_type null_count(Py_ssize_t bitmask, size_type start, size_type stop) +cpdef size_type null_count(gpumemoryview bitmask, size_type start, size_type stop) diff --git a/python/pylibcudf/pylibcudf/null_mask.pyi b/python/pylibcudf/pylibcudf/null_mask.pyi index ace18582bd1a..524b3d432b20 100644 --- a/python/pylibcudf/pylibcudf/null_mask.pyi +++ b/python/pylibcudf/pylibcudf/null_mask.pyi @@ -3,6 +3,7 @@ from rmm.pylibrmm.device_buffer import DeviceBuffer from pylibcudf.column import Column +from pylibcudf.gpumemoryview import gpumemoryview from pylibcudf.types import MaskState def copy_bitmask(col: Column) -> DeviceBuffer: ... @@ -12,4 +13,4 @@ def create_null_mask( ) -> DeviceBuffer: ... def bitmask_and(columns: list[Column]) -> tuple[DeviceBuffer, int]: ... def bitmask_or(columns: list[Column]) -> tuple[DeviceBuffer, int]: ... -def null_count(bitmask: int, start: int, stop: int) -> int: ... +def null_count(bitmask: gpumemoryview, start: int, stop: int) -> int: ... diff --git a/python/pylibcudf/pylibcudf/null_mask.pyx b/python/pylibcudf/pylibcudf/null_mask.pyx index 0260088c0e22..c126bf64f072 100644 --- a/python/pylibcudf/pylibcudf/null_mask.pyx +++ b/python/pylibcudf/pylibcudf/null_mask.pyx @@ -5,6 +5,7 @@ from libcpp.pair cimport pair from libcpp.utility cimport move from pylibcudf.libcudf cimport null_mask as cpp_null_mask from pylibcudf.libcudf.types cimport mask_state, size_type +from pylibcudf.gpumemoryview cimport gpumemoryview from pylibcudf.utils cimport int_to_bitmask_ptr from rmm.librmm.device_buffer cimport device_buffer @@ -152,7 +153,7 @@ cpdef tuple bitmask_or(list columns): return buffer_to_python(move(c_result.first)), c_result.second -cpdef size_type null_count(Py_ssize_t bitmask, size_type start, size_type stop): +cpdef size_type null_count(gpumemoryview bitmask, size_type start, size_type stop): """Given a validity bitmask, counts the number of null elements. For details, see :cpp:func:`null_count`. @@ -172,4 +173,4 @@ cpdef size_type null_count(Py_ssize_t bitmask, size_type start, size_type stop): The number of null elements in the specified range. """ with nogil: - return cpp_null_mask.null_count(int_to_bitmask_ptr(bitmask), start, stop) + return cpp_null_mask.null_count(int_to_bitmask_ptr(bitmask.ptr), start, stop) diff --git a/python/pylibcudf/pylibcudf/tests/test_null_mask.py b/python/pylibcudf/pylibcudf/tests/test_null_mask.py index cd3da856de2c..7f4f524d67ca 100644 --- a/python/pylibcudf/pylibcudf/tests/test_null_mask.py +++ b/python/pylibcudf/pylibcudf/tests/test_null_mask.py @@ -1,4 +1,4 @@ -# Copyright (c) 2024, NVIDIA CORPORATION. +# Copyright (c) 2024-2025, NVIDIA CORPORATION. import pyarrow as pa import pytest @@ -24,11 +24,18 @@ def column(request, nullable): def test_copy_bitmask(column, nullable): - expected = column.null_mask().obj if nullable else rmm.DeviceBuffer() - got = plc.null_mask.copy_bitmask(column) + expected = ( + column.null_mask() + if nullable + else plc.gpumemoryview(rmm.DeviceBuffer()) + ) + got = plc.gpumemoryview(plc.null_mask.copy_bitmask(column)) - assert expected.size == got.size - assert expected.tobytes() == got.tobytes() + start = 0 + end = column.size() * 8 + assert plc.null_mask.null_count( + expected, start, end + ) == plc.null_mask.null_count(got, start, end) def test_bitmask_allocation_size_bytes(): From c89c86b9d4292125b3d74f361efe8bde6aec7f76 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 24 Mar 2025 02:19:14 +0000 Subject: [PATCH 13/41] Remove invalid access to gpumemoryview.obj --- python/cudf/cudf/core/column/column.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/cudf/cudf/core/column/column.py b/python/cudf/cudf/core/column/column.py index c413ca0ef26b..1ef2f7678b17 100644 --- a/python/cudf/cudf/core/column/column.py +++ b/python/cudf/cudf/core/column/column.py @@ -508,12 +508,12 @@ def from_pylibcudf( dtype = dtype_from_pylibcudf_column(col) return cudf.core.column.build_column( # type: ignore[return-value] - data=as_buffer(col.data().obj, exposed=data_ptr_exposed) + data=as_buffer(col.data(), exposed=data_ptr_exposed) if col.data() is not None else None, dtype=dtype, size=col.size(), - mask=as_buffer(col.null_mask().obj, exposed=data_ptr_exposed) + mask=as_buffer(col.null_mask(), exposed=data_ptr_exposed) if col.null_mask() is not None else None, offset=col.offset(), From bbbc422c9ce6d45a739be1aa32ce600ea1368fbb Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 24 Mar 2025 22:55:32 +0000 Subject: [PATCH 14/41] Fix a few more places that need to handle EMPTY --- cpp/src/interop/to_arrow_device.cu | 20 ++++++++++++++++---- cpp/src/interop/to_arrow_schema.cpp | 6 ++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index 277298450293..699d40f86019 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -227,8 +227,14 @@ int dispatch_to_arrow_device::operator()(cudf::column&& colum for (size_t i = 0; i < size_t(tmp->n_children); ++i) { ArrowArray* child_ptr = tmp->children[i]; auto& child = contents.children[i]; - NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( - child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, child_ptr)); + if (child->type().id() == cudf::type_id::EMPTY) { + NANOARROW_RETURN_NOT_OK(initialize_array(tmp->children[0], NANOARROW_TYPE_NA, child->view())); + auto contents = child->release(); + NANOARROW_RETURN_NOT_OK(set_contents(contents, tmp->children[0])); + } else { + NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( + child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, child_ptr)); + } } ArrowArrayMove(tmp.get(), out); @@ -253,8 +259,14 @@ int dispatch_to_arrow_device::operator()(cudf::column&& column, NANOARROW_RETURN_NOT_OK(set_buffer(std::move(offsets_contents.data), 1, tmp.get())); auto& child = contents.children[cudf::lists_column_view::child_column_index]; - NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( - child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, tmp->children[0])); + if (child->type().id() == cudf::type_id::EMPTY) { + NANOARROW_RETURN_NOT_OK(initialize_array(tmp->children[0], NANOARROW_TYPE_NA, child->view())); + auto contents = child->release(); + NANOARROW_RETURN_NOT_OK(set_contents(contents, tmp->children[0])); + } else { + NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( + child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, tmp->children[0])); + } ArrowArrayMove(tmp.get(), out); return NANOARROW_OK; diff --git a/cpp/src/interop/to_arrow_schema.cpp b/cpp/src/interop/to_arrow_schema.cpp index c375bdd0c4c3..1aeb109f3eee 100644 --- a/cpp/src/interop/to_arrow_schema.cpp +++ b/cpp/src/interop/to_arrow_schema.cpp @@ -152,6 +152,9 @@ int dispatch_to_arrow_type::operator()(column_view input, child->flags = col.has_nulls() ? ARROW_FLAG_NULLABLE : 0; + if (col.type().id() == cudf::type_id::EMPTY) { + NANOARROW_RETURN_NOT_OK(ArrowSchemaSetType(out->children[0], NANOARROW_TYPE_NA)); + } NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( col.type(), detail::dispatch_to_arrow_type{}, col, metadata.children_meta[i], child)); } @@ -174,6 +177,9 @@ int dispatch_to_arrow_type::operator()(column_view input, out->flags = input.has_nulls() ? ARROW_FLAG_NULLABLE : 0; NANOARROW_RETURN_NOT_OK(ArrowSchemaSetName(out->children[0], child_meta.name.c_str())); out->children[0]->flags = child.has_nulls() ? ARROW_FLAG_NULLABLE : 0; + if (child.type().id() == cudf::type_id::EMPTY) { + return ArrowSchemaSetType(out->children[0], NANOARROW_TYPE_NA); + } return cudf::type_dispatcher( child.type(), detail::dispatch_to_arrow_type{}, child, child_meta, out->children[0]); } From 83b6b9290ee7396b0c2a3373ce936c2a8f8c2069 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 24 Mar 2025 22:55:59 +0000 Subject: [PATCH 15/41] Update one more use of null_count to match the updated API signature --- python/cudf/cudf/core/column/column.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/cudf/cudf/core/column/column.py b/python/cudf/cudf/core/column/column.py index 1ef2f7678b17..19c058330087 100644 --- a/python/cudf/cudf/core/column/column.py +++ b/python/cudf/cudf/core/column/column.py @@ -329,7 +329,7 @@ def null_count(self) -> int: else: with acquire_spill_lock(): self._null_count = plc.null_mask.null_count( - self.base_mask.get_ptr(mode="read"), # type: ignore[union-attr] + plc.gpumemoryview(self.base_mask), # type: ignore[union-attr] self.offset, self.offset + self.size, ) From e2df6b390311343e11da8e70ca5ff03e8948fb19 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 24 Mar 2025 23:45:59 +0000 Subject: [PATCH 16/41] One more handling of EMPTY types --- cpp/src/interop/to_arrow_schema.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpp/src/interop/to_arrow_schema.cpp b/cpp/src/interop/to_arrow_schema.cpp index 1aeb109f3eee..254a4b8657e1 100644 --- a/cpp/src/interop/to_arrow_schema.cpp +++ b/cpp/src/interop/to_arrow_schema.cpp @@ -154,9 +154,10 @@ int dispatch_to_arrow_type::operator()(column_view input, if (col.type().id() == cudf::type_id::EMPTY) { NANOARROW_RETURN_NOT_OK(ArrowSchemaSetType(out->children[0], NANOARROW_TYPE_NA)); + } else { + NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( + col.type(), detail::dispatch_to_arrow_type{}, col, metadata.children_meta[i], child)); } - NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( - col.type(), detail::dispatch_to_arrow_type{}, col, metadata.children_meta[i], child)); } return NANOARROW_OK; From a820a6c9cdd0be574aeedacb096d6a6454cdc017 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 25 Mar 2025 23:23:43 +0000 Subject: [PATCH 17/41] Fix various typos in null type handling --- cpp/src/interop/to_arrow_device.cu | 10 +++++----- cpp/src/interop/to_arrow_schema.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index 699d40f86019..7b0ef1630043 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -228,9 +228,9 @@ int dispatch_to_arrow_device::operator()(cudf::column&& colum ArrowArray* child_ptr = tmp->children[i]; auto& child = contents.children[i]; if (child->type().id() == cudf::type_id::EMPTY) { - NANOARROW_RETURN_NOT_OK(initialize_array(tmp->children[0], NANOARROW_TYPE_NA, child->view())); - auto contents = child->release(); - NANOARROW_RETURN_NOT_OK(set_contents(contents, tmp->children[0])); + NANOARROW_RETURN_NOT_OK(initialize_array(child_ptr, NANOARROW_TYPE_NA, child->view())); + auto child_contents = child->release(); + NANOARROW_RETURN_NOT_OK(set_contents(child_contents, tmp->children[i])); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, child_ptr)); @@ -261,8 +261,8 @@ int dispatch_to_arrow_device::operator()(cudf::column&& column, auto& child = contents.children[cudf::lists_column_view::child_column_index]; if (child->type().id() == cudf::type_id::EMPTY) { NANOARROW_RETURN_NOT_OK(initialize_array(tmp->children[0], NANOARROW_TYPE_NA, child->view())); - auto contents = child->release(); - NANOARROW_RETURN_NOT_OK(set_contents(contents, tmp->children[0])); + auto child_contents = child->release(); + NANOARROW_RETURN_NOT_OK(set_contents(child_contents, tmp->children[0])); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, tmp->children[0])); diff --git a/cpp/src/interop/to_arrow_schema.cpp b/cpp/src/interop/to_arrow_schema.cpp index 254a4b8657e1..3b5415f02366 100644 --- a/cpp/src/interop/to_arrow_schema.cpp +++ b/cpp/src/interop/to_arrow_schema.cpp @@ -153,7 +153,7 @@ int dispatch_to_arrow_type::operator()(column_view input, child->flags = col.has_nulls() ? ARROW_FLAG_NULLABLE : 0; if (col.type().id() == cudf::type_id::EMPTY) { - NANOARROW_RETURN_NOT_OK(ArrowSchemaSetType(out->children[0], NANOARROW_TYPE_NA)); + NANOARROW_RETURN_NOT_OK(ArrowSchemaSetType(out->children[i], NANOARROW_TYPE_NA)); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( col.type(), detail::dispatch_to_arrow_type{}, col, metadata.children_meta[i], child)); From 8ba32e3c81fb49f984043c1535a388f8c1a3d2ec Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 26 Mar 2025 06:12:00 +0000 Subject: [PATCH 18/41] Revert "Remove invalid access to gpumemoryview.obj" This reverts commit 2d7ea5de01d141a6650068c4c3a513e2bc67b3c8. --- python/cudf/cudf/core/column/column.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/cudf/cudf/core/column/column.py b/python/cudf/cudf/core/column/column.py index 19c058330087..e98625a8d868 100644 --- a/python/cudf/cudf/core/column/column.py +++ b/python/cudf/cudf/core/column/column.py @@ -508,12 +508,12 @@ def from_pylibcudf( dtype = dtype_from_pylibcudf_column(col) return cudf.core.column.build_column( # type: ignore[return-value] - data=as_buffer(col.data(), exposed=data_ptr_exposed) + data=as_buffer(col.data().obj, exposed=data_ptr_exposed) if col.data() is not None else None, dtype=dtype, size=col.size(), - mask=as_buffer(col.null_mask(), exposed=data_ptr_exposed) + mask=as_buffer(col.null_mask().obj, exposed=data_ptr_exposed) if col.null_mask() is not None else None, offset=col.offset(), From 69fd71f6c69cf023dfff82466c2645a723a84d33 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 26 Mar 2025 06:26:28 +0000 Subject: [PATCH 19/41] Wrap column views in objects that expose the CUDA array interface and use those to construct gpumemoryviews safely --- python/pylibcudf/pylibcudf/column.pxd | 16 ++++ python/pylibcudf/pylibcudf/column.pyx | 82 ++++++++++++++++++-- python/pylibcudf/pylibcudf/gpumemoryview.pxd | 4 +- python/pylibcudf/pylibcudf/gpumemoryview.pyx | 23 +----- 4 files changed, 93 insertions(+), 32 deletions(-) diff --git a/python/pylibcudf/pylibcudf/column.pxd b/python/pylibcudf/pylibcudf/column.pxd index 03cbbd7c2067..1f31233a835b 100644 --- a/python/pylibcudf/pylibcudf/column.pxd +++ b/python/pylibcudf/pylibcudf/column.pxd @@ -14,6 +14,22 @@ from .gpumemoryview cimport gpumemoryview from .types cimport DataType +cdef class OwnerWithCAI: + cdef object owner + cdef dict cai + + @staticmethod + cdef create(column_view cv, object owner) + + +cdef class OwnerMaskWithCAI: + cdef object owner + cdef dict cai + + @staticmethod + cdef create(column_view cv, object owner) + + cdef class Column: # TODO: Should we document these attributes? Should we mark them readonly? cdef: diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index 4221e0b0fab8..3d1189b6c1d4 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -11,8 +11,10 @@ from libcpp.utility cimport move from pylibcudf.libcudf.column.column cimport column, column_contents from pylibcudf.libcudf.column.column_factories cimport make_column_from_scalar -from pylibcudf.libcudf.scalar.scalar cimport scalar -from pylibcudf.libcudf.types cimport size_type +from pylibcudf.libcudf.scalar.scalar cimport scalar, numeric_scalar +from pylibcudf.libcudf.types cimport size_type, size_of as cpp_size_of +from pylibcudf.libcudf.utilities.traits cimport is_fixed_width, is_fixed_point +from pylibcudf.libcudf.copying cimport get_element from rmm.pylibrmm.device_buffer cimport DeviceBuffer @@ -26,6 +28,7 @@ from .gpumemoryview cimport gpumemoryview from .scalar cimport Scalar from .types cimport DataType, size_of, type_id from .utils cimport int_to_bitmask_ptr, int_to_void_ptr +from .null_mask cimport bitmask_allocation_size_bytes import functools @@ -45,6 +48,71 @@ cdef class _ArrowColumnHolder: cdef unique_ptr[arrow_column] col +cdef class OwnerWithCAI: + @staticmethod + cdef create(column_view cv, object owner): + obj = OwnerWithCAI() + obj.owner = owner + cdef int size + cdef column_view offsets_column + cdef unique_ptr[scalar] last_offset + if cv.type().id() == type_id.EMPTY: + size = cv.size() + elif is_fixed_width(cv.type()) or is_fixed_point(cv.type()): + size = cv.size() * cpp_size_of(cv.type()) + elif cv.type().id() == type_id.STRING: + # The size of the character array in the parent is the offsets size + num_children = cv.num_children() + size = 0 + # A strings column with no children is created for empty/all null + if num_children: + offsets_column = cv.child(0) + last_offset = get_element(offsets_column, offsets_column.size() - 1) + size = ( last_offset.get()).value() + else: + # All other types store data in the children, so the parent size is 0 + size = 0 + + obj.cai = { + "shape": (size,), + "strides": None, + # For the purposes in this function, just treat all of the types as byte + # streams of the appropriate size. This matches what we currently get from + # rmm.DeviceBuffer + "typestr": "|u1", + "data": ( cv.head[char](), False), + "version": 3, + } + return obj + + @property + def __cuda_array_interface__(self): + return self.cai + + +cdef class OwnerMaskWithCAI: + @staticmethod + cdef create(column_view cv, object owner): + obj = OwnerMaskWithCAI() + obj.owner = owner + + obj.cai = { + "shape": (bitmask_allocation_size_bytes(cv.size()),), + "strides": None, + # For the purposes in this function, just treat all of the types as byte + # streams of the appropriate size. This matches what we currently get from + # rmm.DeviceBuffer + "typestr": "|u1", + "data": ( cv.null_mask(), False), + "version": 3, + } + return obj + + @property + def __cuda_array_interface__(self): + return self.cai + + cdef class Column: """A container of nullable device data as a column of elements. @@ -316,12 +384,10 @@ cdef class Column: Column.from_column_view_of_arbitrary(cv.child(i), owner) ) - cdef gpumemoryview owning_data = gpumemoryview.from_pointer( - cv.head[char](), owner - ) - cdef gpumemoryview owning_mask = gpumemoryview.from_pointer( - cv.null_mask(), owner - ) + cdef gpumemoryview owning_data = gpumemoryview(OwnerWithCAI.create(cv, owner)) + cdef gpumemoryview owning_mask = None + if cv.null_count() > 0: + owning_mask = gpumemoryview(OwnerMaskWithCAI.create(cv, owner)) return Column( DataType.from_libcudf(cv.type()), diff --git a/python/pylibcudf/pylibcudf/gpumemoryview.pxd b/python/pylibcudf/pylibcudf/gpumemoryview.pxd index be7fc4718168..47d0dfcfdbde 100644 --- a/python/pylibcudf/pylibcudf/gpumemoryview.pxd +++ b/python/pylibcudf/pylibcudf/gpumemoryview.pxd @@ -6,6 +6,4 @@ cdef class gpumemoryview: # to treat this object as something like a POD struct cdef readonly Py_ssize_t ptr cdef readonly object obj - - @staticmethod - cdef gpumemoryview from_pointer(Py_ssize_t ptr, object owner) + cdef readonly dict cai diff --git a/python/pylibcudf/pylibcudf/gpumemoryview.pyx b/python/pylibcudf/pylibcudf/gpumemoryview.pyx index 39f8c6f95904..6c13cac4f3f0 100644 --- a/python/pylibcudf/pylibcudf/gpumemoryview.pyx +++ b/python/pylibcudf/pylibcudf/gpumemoryview.pyx @@ -23,32 +23,13 @@ cdef class gpumemoryview: "the CUDA array interface" ) self.obj = obj + self.cai = cai # TODO: Need to respect readonly self.ptr = cai["data"][0] - @staticmethod - cdef gpumemoryview from_pointer(Py_ssize_t ptr, object owner): - """Create a gpumemoryview from a pointer and an owning object. - - Parameters - ---------- - ptr : Py_ssize_t - The pointer to the memory. - owner : object - The object that owns the data the pointer points to. - - Returns - ------- - gpumemoryview - """ - cdef gpumemoryview out = gpumemoryview.__new__(gpumemoryview) - out.obj = owner - out.ptr = ptr - return out - @property def __cuda_array_interface__(self): - return self.obj.__cuda_array_interface__ + return self.cai def __len__(self): return self.obj.__cuda_array_interface__["shape"][0] From 08ae13a666301bccf202d052f080ce9e59355be3 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 1 Apr 2025 17:36:45 +0000 Subject: [PATCH 20/41] Update pytest-benchmark bound --- conda/environments/all_cuda-118_arch-x86_64.yaml | 2 +- conda/environments/all_cuda-128_arch-x86_64.yaml | 2 +- dependencies.yaml | 6 +++++- python/cudf/pyproject.toml | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index f68c75f104d5..2328fced67d3 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -72,7 +72,7 @@ dependencies: - pyarrow>=14.0.0,<20.0.0a0 - pydata-sphinx-theme>=0.15.4 - pynvml>=12.0.0,<13.0.0a0 -- pytest-benchmark +- pytest-benchmark<5.1.0 - pytest-cases>=3.8.2 - pytest-cov - pytest-xdist diff --git a/conda/environments/all_cuda-128_arch-x86_64.yaml b/conda/environments/all_cuda-128_arch-x86_64.yaml index 7b09db0e61a8..f47d55450887 100644 --- a/conda/environments/all_cuda-128_arch-x86_64.yaml +++ b/conda/environments/all_cuda-128_arch-x86_64.yaml @@ -70,7 +70,7 @@ dependencies: - pydata-sphinx-theme>=0.15.4 - pynvjitlink>=0.0.0a0 - pynvml>=12.0.0,<13.0.0a0 -- pytest-benchmark +- pytest-benchmark<5.1.0 - pytest-cases>=3.8.2 - pytest-cov - pytest-xdist diff --git a/dependencies.yaml b/dependencies.yaml index 6668be32addf..242b56e232d4 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -929,7 +929,11 @@ dependencies: - cramjam - fastavro>=0.22.9 - hypothesis - - pytest-benchmark + # Version 5.1 is incompatible with pytest<8.2. + # https://github.com/ionelmc/pytest-benchmark/commit/8dfeeeca8a5c640a4dc4455904ace4f97f62e655 + # Remove upper bound when we unbound pytest + # https://github.com/rapidsai/build-planning/issues/105 + - pytest-benchmark<5.1.0 - pytest-cases>=3.8.2 - scipy - mmh3 diff --git a/python/cudf/pyproject.toml b/python/cudf/pyproject.toml index b603ccefe5b2..9d9643eb777d 100644 --- a/python/cudf/pyproject.toml +++ b/python/cudf/pyproject.toml @@ -56,7 +56,7 @@ test = [ "hypothesis", "mmh3", "msgpack", - "pytest-benchmark", + "pytest-benchmark<5.1.0", "pytest-cases>=3.8.2", "pytest-cov", "pytest-xdist", From 2197a21e4747f6e074515d90cdd5cee12f16ce39 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 1 Apr 2025 17:39:50 +0000 Subject: [PATCH 21/41] Add a benchmark using pyarrow for construction --- python/cudf/benchmarks/API/bench_dataframe.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/python/cudf/benchmarks/API/bench_dataframe.py b/python/cudf/benchmarks/API/bench_dataframe.py index ba243eb6a7c5..5920aa9cb846 100644 --- a/python/cudf/benchmarks/API/bench_dataframe.py +++ b/python/cudf/benchmarks/API/bench_dataframe.py @@ -1,4 +1,4 @@ -# Copyright (c) 2022-2024, NVIDIA CORPORATION. +# Copyright (c) 2022-2025, NVIDIA CORPORATION. """Benchmarks of DataFrame methods.""" @@ -6,12 +6,19 @@ import numba.cuda import numpy +import pyarrow as pa import pytest import pytest_cases from config import cudf, cupy from utils import benchmark_with_object +@pytest.mark.parametrize("N", [100, 1_000_000, 100_000_000]) +def bench_from_arrow(benchmark, N): + rng = numpy.random.default_rng(seed=10) + benchmark(cudf.DataFrame, {None: pa.array(rng.random(N))}) + + @pytest.mark.parametrize("N", [100, 1_000_000]) def bench_construction(benchmark, N): benchmark(cudf.DataFrame, {None: cupy.random.rand(N)}) From 3eb72eef21cce703782e935d93ece3334e4786d7 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 1 Apr 2025 18:11:04 +0000 Subject: [PATCH 22/41] Fix comments --- python/pylibcudf/pylibcudf/table.pyx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 08c885cea1e4..b6640c7a4f64 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -226,7 +226,8 @@ cdef class Table: def _create_nested_column_metadata(Column col): # TODO: We'll need to reshuffle where things are defined to avoid circular # imports. For now, we'll just import this inline. We should be able to avoid - # circularity altogether by simply + # circularity altogether by simply not needing ColumnMetadata at all in the + # future and just using the schema directly, so we can consider that approach. from pylibcudf.interop import ColumnMetadata return ColumnMetadata( children_meta=[ @@ -238,7 +239,8 @@ cdef class Table: """Create an Arrow schema from this table.""" # TODO: We'll need to reshuffle where things are defined to avoid circular # imports. For now, we'll just import this inline. We should be able to avoid - # circularity altogether by simply + # circularity altogether by simply not needing ColumnMetadata at all in the + # future and just using the schema directly, so we can consider that approach. from pylibcudf.interop import ColumnMetadata if metadata is None: metadata = [ From f07f8cbcd2f6440f98fb30d5dce1f07d342f0c57 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 1 Apr 2025 18:48:19 +0000 Subject: [PATCH 23/41] More comments --- dependencies.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dependencies.yaml b/dependencies.yaml index 242b56e232d4..03da99e1c717 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -933,6 +933,9 @@ dependencies: # https://github.com/ionelmc/pytest-benchmark/commit/8dfeeeca8a5c640a4dc4455904ace4f97f62e655 # Remove upper bound when we unbound pytest # https://github.com/rapidsai/build-planning/issues/105 + # or when a fixed version of pytest-benchmark 5.1 with the necessary bounds is released + # https://github.com/conda-forge/pytest-benchmark-feedstock/pull/27 + # https://github.com/conda-forge/conda-forge-repodata-patches-feedstock/pull/990 - pytest-benchmark<5.1.0 - pytest-cases>=3.8.2 - scipy From 281ece37b5a967233e6dd2d3398255effbdd111e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 9 Apr 2025 22:06:28 +0000 Subject: [PATCH 24/41] Expose the protocol for columns as well and make column implementation independent of table --- python/pylibcudf/pylibcudf/column.pyx | 76 +++++++++++++++++++ python/pylibcudf/pylibcudf/interop.pxd | 3 +- python/pylibcudf/pylibcudf/interop.pyx | 25 +++--- .../pylibcudf/pylibcudf/libcudf/interop.pxd | 29 +++++++ python/pylibcudf/pylibcudf/table.pyx | 54 +++++-------- .../pylibcudf/tests/test_labeling.py | 4 +- .../pylibcudf/tests/test_string_extract.py | 6 +- .../pylibcudf/tests/test_string_padding.py | 6 +- .../pylibcudf/tests/test_string_repeat.py | 4 +- .../pylibcudf/tests/test_transform.py | 4 +- 10 files changed, 153 insertions(+), 58 deletions(-) diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index cd22ff42be5c..c404d34091bb 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -4,6 +4,7 @@ from cython.operator cimport dereference from cpython.pycapsule cimport ( PyCapsule_GetPointer, + PyCapsule_New, ) from libcpp.limits cimport numeric_limits @@ -18,6 +19,16 @@ from pylibcudf.libcudf.types cimport size_type, size_of as cpp_size_of from pylibcudf.libcudf.utilities.traits cimport is_fixed_width, is_fixed_point from pylibcudf.libcudf.copying cimport get_element +from pylibcudf.libcudf.interop cimport ( + ArrowArray, + ArrowSchema, + column_metadata, + release_arrow_array_raw, + release_arrow_schema_raw, + to_arrow_host_raw, + to_arrow_schema_raw, +) + from rmm.pylibrmm.device_buffer cimport DeviceBuffer from rmm.pylibrmm.stream cimport Stream @@ -35,6 +46,38 @@ import functools __all__ = ["Column", "ListColumnView", "is_c_contiguous"] +cdef void _release_schema(object schema_capsule) noexcept: + """Release the ArrowSchema object stored in a PyCapsule.""" + cdef ArrowSchema* schema = PyCapsule_GetPointer( + schema_capsule, 'arrow_schema' + ) + release_arrow_schema_raw(schema) + + +cdef void _release_array(object array_capsule) noexcept: + """Release the ArrowArray object stored in a PyCapsule.""" + cdef ArrowArray* array = PyCapsule_GetPointer( + array_capsule, 'arrow_array' + ) + release_arrow_array_raw(array) + + +cdef column_metadata _metadata_to_libcudf(metadata): + """Convert a ColumnMetadata object to C++ column_metadata. + + Since this class is mutable and cheap, it is easier to create the C++ + object on the fly rather than have it directly backing the storage for + the Cython class. Additionally, this structure restricts the dependency + on C++ types to just within this module, allowing us to make the module a + pure Python module (from an import sense, i.e. no pxd declarations). + """ + cdef column_metadata c_metadata + c_metadata.name = metadata.name.encode() + for child_meta in metadata.children_meta: + c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) + return c_metadata + + class _ArrowLikeMeta(type): def __subclasscheck__(cls, other): return hasattr(other, "__arrow_c_array__") @@ -661,6 +704,39 @@ cdef class Column: c_result = make_unique[column](self.view()) return Column.from_libcudf(move(c_result)) + def _to_schema(self, metadata=None): + """Create an Arrow schema from this Column.""" + # TODO: We'll need to reshuffle where things are defined to avoid circular + # imports. For now, we'll just import this inline. We should be able to avoid + # circularity altogether by simply not needing ColumnMetadata at all in the + # future and just using the schema directly, so we can consider that approach. + from pylibcudf.interop import ColumnMetadata, _create_nested_column_metadata + if metadata is None: + metadata = _create_nested_column_metadata(self) + elif isinstance(metadata, str): + metadata = ColumnMetadata(metadata) + + cdef column_metadata c_metadata = _metadata_to_libcudf(metadata) + + cdef ArrowSchema* raw_schema_ptr + with nogil: + raw_schema_ptr = to_arrow_schema_raw(self.view(), c_metadata) + + return PyCapsule_New(raw_schema_ptr, 'arrow_schema', _release_schema) + + def _to_host_array(self): + cdef ArrowArray* raw_host_array_ptr + with nogil: + raw_host_array_ptr = to_arrow_host_raw(self.view()) + + return PyCapsule_New(raw_host_array_ptr, "arrow_array", _release_array) + + def __arrow_c_array__(self, requested_schema=None): + if requested_schema is not None: + raise ValueError("pylibcudf.Column does not support alternative schema") + + return self._to_schema(), self._to_host_array() + cdef class ListColumnView: """Accessor for methods of a Column that are specific to lists.""" diff --git a/python/pylibcudf/pylibcudf/interop.pxd b/python/pylibcudf/pylibcudf/interop.pxd index 2a0a8c15fdd2..ccbfb8e928c1 100644 --- a/python/pylibcudf/pylibcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/interop.pxd @@ -1,7 +1,8 @@ -# Copyright (c) 2024, NVIDIA CORPORATION. +# Copyright (c) 2024-2025, NVIDIA CORPORATION. from pylibcudf.table cimport Table +from pylibcudf.libcudf.interop cimport column_metadata cpdef Table from_dlpack(object managed_tensor) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 6c34799f57a7..e712c987f100 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -75,6 +75,14 @@ class ColumnMetadata: children_meta: list[ColumnMetadata] = field(default_factory=list) +def _create_nested_column_metadata(Column col): + return ColumnMetadata( + children_meta=[ + _create_nested_column_metadata(child) for child in col.children() + ] + ) + + @singledispatch def from_arrow(pyarrow_object, *, DataType data_type=None): """Create a cudf object from a pyarrow object. @@ -198,29 +206,28 @@ def _to_arrow_datatype(cudf_object, **kwargs): ) -class _TableWithArrowMetadata: - def __init__(self, tbl, metadata=None): - self.tbl = tbl +class _ObjectWithArrowMetadata: + def __init__(self, obj, metadata=None): + self.obj = obj self.metadata = metadata def __arrow_c_array__(self, requested_schema=None): - return self.tbl._to_schema(self.metadata), self.tbl._to_host_array() + return self.obj._to_schema(self.metadata), self.obj._to_host_array() @to_arrow.register(Table) def _to_arrow_table(cudf_object, metadata=None): + """Create a PyArrow table from a pylibcudf table.""" # TODO: See if we can stop supporting configuration of struct field names when # exporting to arrow data. That would allow us to get rid of the - # _TableWithArrowMetadata struct and just use the underlying Table directly. - return pa.table(_TableWithArrowMetadata(cudf_object, metadata)) + # _ObjectWithArrowMetadata struct and just use the underlying Table directly. + return pa.table(_ObjectWithArrowMetadata(cudf_object, metadata)) @to_arrow.register(Column) def _to_arrow_array(cudf_object, metadata=None): """Create a PyArrow array from a pylibcudf column.""" - if metadata is not None: - metadata = [metadata] - return to_arrow(Table([cudf_object]), metadata)[0] + return pa.array(_ObjectWithArrowMetadata(cudf_object, metadata)) @to_arrow.register(Scalar) diff --git a/python/pylibcudf/pylibcudf/libcudf/interop.pxd b/python/pylibcudf/pylibcudf/libcudf/interop.pxd index 256478eb17d2..8adc9827047f 100644 --- a/python/pylibcudf/pylibcudf/libcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/libcudf/interop.pxd @@ -80,6 +80,17 @@ cdef extern from *: return to_arrow_schema(input, metadata).release(); } + ArrowSchema* to_arrow_schema_raw( + cudf::column_view const& input, + cudf::column_metadata const& metadata) { + std::vector metadata_vec{metadata}; + cudf::table_view const& tbl = cudf::table_view({input}); + auto schema = cudf::to_arrow_schema(tbl, metadata_vec); + ArrowSchema *array_schema = new ArrowSchema(); + ArrowSchemaMove(schema->children[0], array_schema); + return array_schema; + } + void release_arrow_schema_raw(ArrowSchema *schema) { if (schema->release != nullptr) { schema->release(schema); @@ -98,6 +109,17 @@ cdef extern from *: return arr; } + ArrowArray* to_arrow_host_raw( + cudf::column_view const& col, + rmm::cuda_stream_view stream = cudf::get_default_stream(), + rmm::device_async_resource_ref mr = cudf::get_current_device_resource_ref()) { + // Assumes the sync event is null and the data is already on the host. + ArrowArray *arr = new ArrowArray(); + auto device_arr = cudf::to_arrow_host(col, stream, mr); + ArrowArrayMove(&device_arr->array, arr); + return arr; + } + void release_arrow_array_raw(ArrowArray *array) { if (array->release != nullptr) { array->release(array); @@ -109,12 +131,19 @@ cdef extern from *: const table_view& tbl, const vector[column_metadata]& metadata, ) except +libcudf_exception_handler nogil + cdef ArrowSchema *to_arrow_schema_raw( + const column_view& tbl, + const column_metadata& metadata, + ) except +libcudf_exception_handler nogil cdef void release_arrow_schema_raw( ArrowSchema * ) except +libcudf_exception_handler nogil cdef ArrowArray* to_arrow_host_raw( const table_view& tbl ) except +libcudf_exception_handler nogil + cdef ArrowArray* to_arrow_host_raw( + const column_view& tbl + ) except +libcudf_exception_handler nogil cdef void release_arrow_array_raw( ArrowArray * ) except +libcudf_exception_handler nogil diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 3bb69e023e57..5e458070897a 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -36,23 +36,6 @@ from functools import singledispatchmethod __all__ = ["Table"] -# TODO: Add a strong type here on the ColumnMetadata input -cdef column_metadata _metadata_to_libcudf(metadata): - """Convert a ColumnMetadata object to C++ column_metadata. - - Since this class is mutable and cheap, it is easier to create the C++ - object on the fly rather than have it directly backing the storage for - the Cython class. Additionally, this structure restricts the dependency - on C++ types to just within this module, allowing us to make the module a - pure Python module (from an import sense, i.e. no pxd declarations). - """ - cdef column_metadata c_metadata - c_metadata.name = metadata.name.encode() - for child_meta in metadata.children_meta: - c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) - return c_metadata - - cdef void _release_schema(object schema_capsule) noexcept: """Release the ArrowSchema object stored in a PyCapsule.""" cdef ArrowSchema* schema = PyCapsule_GetPointer( @@ -69,6 +52,22 @@ cdef void _release_array(object array_capsule) noexcept: release_arrow_array_raw(array) +cdef column_metadata _metadata_to_libcudf(metadata): + """Convert a ColumnMetadata object to C++ column_metadata. + + Since this class is mutable and cheap, it is easier to create the C++ + object on the fly rather than have it directly backing the storage for + the Cython class. Additionally, this structure restricts the dependency + on C++ types to just within this module, allowing us to make the module a + pure Python module (from an import sense, i.e. no pxd declarations). + """ + cdef column_metadata c_metadata + c_metadata.name = metadata.name.encode() + for child_meta in metadata.children_meta: + c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) + return c_metadata + + class _ArrowLikeMeta(type): # Unfortunately we cannot separate stream and array via singledispatch because the # dispatch will often be ambiguous when objects expose both protocols. @@ -220,29 +219,16 @@ cdef class Table: """The columns in this table.""" return self._columns - @staticmethod - def _create_nested_column_metadata(Column col): - # TODO: We'll need to reshuffle where things are defined to avoid circular - # imports. For now, we'll just import this inline. We should be able to avoid - # circularity altogether by simply not needing ColumnMetadata at all in the - # future and just using the schema directly, so we can consider that approach. - from pylibcudf.interop import ColumnMetadata - return ColumnMetadata( - children_meta=[ - Table._create_nested_column_metadata(child) for child in col.children() - ] - ) - def _to_schema(self, metadata=None): """Create an Arrow schema from this table.""" # TODO: We'll need to reshuffle where things are defined to avoid circular # imports. For now, we'll just import this inline. We should be able to avoid # circularity altogether by simply not needing ColumnMetadata at all in the # future and just using the schema directly, so we can consider that approach. - from pylibcudf.interop import ColumnMetadata + from pylibcudf.interop import ColumnMetadata, _create_nested_column_metadata if metadata is None: metadata = [ - Table._create_nested_column_metadata(col) for col in self.columns() + _create_nested_column_metadata(col) for col in self.columns() ] else: metadata = [ @@ -271,6 +257,4 @@ cdef class Table: if requested_schema is not None: raise ValueError("pylibcudf.Table does not support alternative schema") - # For the host array protocol the capsules own the data. - ret = self._to_schema(), self._to_host_array() - return ret + return self._to_schema(), self._to_host_array() diff --git a/python/pylibcudf/pylibcudf/tests/test_labeling.py b/python/pylibcudf/pylibcudf/tests/test_labeling.py index 946d583d1cc2..ec9a743a4152 100644 --- a/python/pylibcudf/pylibcudf/tests/test_labeling.py +++ b/python/pylibcudf/pylibcudf/tests/test_labeling.py @@ -1,4 +1,4 @@ -# Copyright (c) 2024, NVIDIA CORPORATION. +# Copyright (c) 2024-2025, NVIDIA CORPORATION. import pyarrow as pa import pytest @@ -21,7 +21,7 @@ def test_label_bins(left_inclusive, right_inclusive): in_col, left_edges, left_inclusive, right_edges, right_inclusive ) ) - expected = pa.chunked_array([[0, 0, 0]], type=pa.int32()) + expected = pa.array([0, 0, 0], type=pa.int32()) assert result.equals(expected) diff --git a/python/pylibcudf/pylibcudf/tests/test_string_extract.py b/python/pylibcudf/pylibcudf/tests/test_string_extract.py index e70edf4fb339..2b395edc2427 100644 --- a/python/pylibcudf/pylibcudf/tests/test_string_extract.py +++ b/python/pylibcudf/pylibcudf/tests/test_string_extract.py @@ -1,4 +1,4 @@ -# Copyright (c) 2024, NVIDIA CORPORATION. +# Copyright (c) 2024-2025, NVIDIA CORPORATION. import pyarrow as pa import pyarrow.compute as pc @@ -33,7 +33,5 @@ def test_extract_all_record(): ), ) result = plc.interop.to_arrow(plc_result) - expected = pa.chunked_array( - [pa.array([["a", "1"], ["b", "2"], None], type=result.type)] - ) + expected = pa.array([["a", "1"], ["b", "2"], None], type=result.type) assert result.equals(expected) diff --git a/python/pylibcudf/pylibcudf/tests/test_string_padding.py b/python/pylibcudf/pylibcudf/tests/test_string_padding.py index 794981320979..d2c69790bc6f 100644 --- a/python/pylibcudf/pylibcudf/tests/test_string_padding.py +++ b/python/pylibcudf/pylibcudf/tests/test_string_padding.py @@ -1,4 +1,4 @@ -# Copyright (c) 2024, NVIDIA CORPORATION. +# Copyright (c) 2024-2025, NVIDIA CORPORATION. import pyarrow as pa import pyarrow.compute as pc @@ -15,7 +15,7 @@ def test_pad(): "!", ) result = plc.interop.to_arrow(plc_result) - expected = pa.chunked_array(pc.utf8_lpad(arr, 2, padding="!")) + expected = pa.array(pc.utf8_lpad(arr, 2, padding="!")) assert result.equals(expected) @@ -23,5 +23,5 @@ def test_zfill(): arr = pa.array(["a", "1", None]) plc_result = plc.strings.padding.zfill(plc.interop.from_arrow(arr), 2) result = plc.interop.to_arrow(plc_result) - expected = pa.chunked_array(pc.utf8_lpad(arr, 2, padding="0")) + expected = pa.array(pc.utf8_lpad(arr, 2, padding="0")) assert result.equals(expected) diff --git a/python/pylibcudf/pylibcudf/tests/test_string_repeat.py b/python/pylibcudf/pylibcudf/tests/test_string_repeat.py index c06c06be7c64..1c5f318fabd1 100644 --- a/python/pylibcudf/pylibcudf/tests/test_string_repeat.py +++ b/python/pylibcudf/pylibcudf/tests/test_string_repeat.py @@ -1,4 +1,4 @@ -# Copyright (c) 2024, NVIDIA CORPORATION. +# Copyright (c) 2024-2025, NVIDIA CORPORATION. import pyarrow as pa import pyarrow.compute as pc @@ -17,5 +17,5 @@ def test_repeat_strings(repeats): else repeats, ) result = plc.interop.to_arrow(plc_result) - expected = pa.chunked_array(pc.binary_repeat(arr, repeats)) + expected = pa.array(pc.binary_repeat(arr, repeats)) assert result.equals(expected) diff --git a/python/pylibcudf/pylibcudf/tests/test_transform.py b/python/pylibcudf/pylibcudf/tests/test_transform.py index a63d94678ff8..d6354508d9d5 100644 --- a/python/pylibcudf/pylibcudf/tests/test_transform.py +++ b/python/pylibcudf/pylibcudf/tests/test_transform.py @@ -46,7 +46,7 @@ def test_bools_to_mask_roundtrip(): plc_output = plc.transform.mask_to_bools(mask.ptr, 0, len(pa_array)) result_pa = plc.interop.to_arrow(plc_output) - expected_pa = pa.chunked_array([[True, False, False]]) + expected_pa = pa.array([True, False, False]) assert result_pa.equals(expected_pa) @@ -68,7 +68,7 @@ def test_encode(): ) assert pa_table_result.equals(pa_table_expected) - pa_column_expected = pa.chunked_array([[0, 1, 2]], type=pa.int32()) + pa_column_expected = pa.array([0, 1, 2], type=pa.int32()) assert pa_column_result.equals(pa_column_expected) From b3f29ebb1653fc258e14d7e4313589b4c6a0bfa9 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 9 Apr 2025 22:51:13 +0000 Subject: [PATCH 25/41] Centralize common interop functions --- python/pylibcudf/pylibcudf/CMakeLists.txt | 2 + python/pylibcudf/pylibcudf/_interop.pxd | 9 ++++ python/pylibcudf/pylibcudf/_interop.pyx | 55 +++++++++++++++++++++++ python/pylibcudf/pylibcudf/column.pyx | 54 ++++++---------------- python/pylibcudf/pylibcudf/interop.pxd | 2 - python/pylibcudf/pylibcudf/interop.pyx | 20 +-------- python/pylibcudf/pylibcudf/table.pyx | 49 ++++---------------- 7 files changed, 89 insertions(+), 102 deletions(-) create mode 100644 python/pylibcudf/pylibcudf/_interop.pxd create mode 100644 python/pylibcudf/pylibcudf/_interop.pyx diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index 9aea24bb410f..a267ee249ebc 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -28,6 +28,7 @@ set(cython_sources groupby.pyx hashing.pyx interop.pyx + _interop.pyx jit.pyx join.pyx json.pyx @@ -68,6 +69,7 @@ include(../../../cpp/cmake/thirdparty/get_nanoarrow.cmake) target_link_libraries(pylibcudf_interop PUBLIC nanoarrow) target_link_libraries(pylibcudf_table PUBLIC nanoarrow) target_link_libraries(pylibcudf_column PUBLIC nanoarrow) +target_link_libraries(pylibcudf__interop PUBLIC nanoarrow) add_subdirectory(libcudf) add_subdirectory(strings) diff --git a/python/pylibcudf/pylibcudf/_interop.pxd b/python/pylibcudf/pylibcudf/_interop.pxd new file mode 100644 index 000000000000..72036be4e775 --- /dev/null +++ b/python/pylibcudf/pylibcudf/_interop.pxd @@ -0,0 +1,9 @@ +# Copyright (c) 2025, NVIDIA CORPORATION. + +from pylibcudf.libcudf.interop cimport column_metadata + +cdef void _release_schema(object schema_capsule) noexcept + +cdef void _release_array(object array_capsule) noexcept + +cdef column_metadata _metadata_to_libcudf(metadata) diff --git a/python/pylibcudf/pylibcudf/_interop.pyx b/python/pylibcudf/pylibcudf/_interop.pyx new file mode 100644 index 000000000000..f2fa6ccf549d --- /dev/null +++ b/python/pylibcudf/pylibcudf/_interop.pyx @@ -0,0 +1,55 @@ +# Copyright (c) 2025, NVIDIA CORPORATION. + +from cpython.pycapsule cimport PyCapsule_GetPointer + +from pylibcudf.libcudf.interop cimport ( + ArrowArray, + ArrowSchema, + column_metadata, + release_arrow_array_raw, + release_arrow_schema_raw, +) + +from dataclasses import dataclass, field + + +@dataclass +class ColumnMetadata: + """Metadata associated with a column. + + This is the Python representation of :cpp:class:`cudf::column_metadata`. + """ + name: str = "" + children_meta: list[ColumnMetadata] = field(default_factory=list) + + +cdef void _release_schema(object schema_capsule) noexcept: + """Release the ArrowSchema object stored in a PyCapsule.""" + cdef ArrowSchema* schema = PyCapsule_GetPointer( + schema_capsule, 'arrow_schema' + ) + release_arrow_schema_raw(schema) + + +cdef void _release_array(object array_capsule) noexcept: + """Release the ArrowArray object stored in a PyCapsule.""" + cdef ArrowArray* array = PyCapsule_GetPointer( + array_capsule, 'arrow_array' + ) + release_arrow_array_raw(array) + + +cdef column_metadata _metadata_to_libcudf(metadata): + """Convert a ColumnMetadata object to C++ column_metadata. + + Since this class is mutable and cheap, it is easier to create the C++ + object on the fly rather than have it directly backing the storage for + the Cython class. Additionally, this structure restricts the dependency + on C++ types to just within this module, allowing us to make the module a + pure Python module (from an import sense, i.e. no pxd declarations). + """ + cdef column_metadata c_metadata + c_metadata.name = metadata.name.encode() + for child_meta in metadata.children_meta: + c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) + return c_metadata diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index c404d34091bb..c2258e6f55a2 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -23,8 +23,6 @@ from pylibcudf.libcudf.interop cimport ( ArrowArray, ArrowSchema, column_metadata, - release_arrow_array_raw, - release_arrow_schema_raw, to_arrow_host_raw, to_arrow_schema_raw, ) @@ -39,6 +37,12 @@ from .scalar cimport Scalar from .types cimport DataType, size_of, type_id from .utils cimport int_to_bitmask_ptr, int_to_void_ptr, _get_stream from .null_mask cimport bitmask_allocation_size_bytes +from ._interop cimport ( + _release_schema, + _release_array, + _metadata_to_libcudf, +) +from ._interop import ColumnMetadata import functools @@ -46,38 +50,6 @@ import functools __all__ = ["Column", "ListColumnView", "is_c_contiguous"] -cdef void _release_schema(object schema_capsule) noexcept: - """Release the ArrowSchema object stored in a PyCapsule.""" - cdef ArrowSchema* schema = PyCapsule_GetPointer( - schema_capsule, 'arrow_schema' - ) - release_arrow_schema_raw(schema) - - -cdef void _release_array(object array_capsule) noexcept: - """Release the ArrowArray object stored in a PyCapsule.""" - cdef ArrowArray* array = PyCapsule_GetPointer( - array_capsule, 'arrow_array' - ) - release_arrow_array_raw(array) - - -cdef column_metadata _metadata_to_libcudf(metadata): - """Convert a ColumnMetadata object to C++ column_metadata. - - Since this class is mutable and cheap, it is easier to create the C++ - object on the fly rather than have it directly backing the storage for - the Cython class. Additionally, this structure restricts the dependency - on C++ types to just within this module, allowing us to make the module a - pure Python module (from an import sense, i.e. no pxd declarations). - """ - cdef column_metadata c_metadata - c_metadata.name = metadata.name.encode() - for child_meta in metadata.children_meta: - c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) - return c_metadata - - class _ArrowLikeMeta(type): def __subclasscheck__(cls, other): return hasattr(other, "__arrow_c_array__") @@ -704,15 +676,17 @@ cdef class Column: c_result = make_unique[column](self.view()) return Column.from_libcudf(move(c_result)) + def _create_nested_column_metadata(self): + return ColumnMetadata( + children_meta=[ + child._create_nested_column_metadata() for child in self.children() + ] + ) + def _to_schema(self, metadata=None): """Create an Arrow schema from this Column.""" - # TODO: We'll need to reshuffle where things are defined to avoid circular - # imports. For now, we'll just import this inline. We should be able to avoid - # circularity altogether by simply not needing ColumnMetadata at all in the - # future and just using the schema directly, so we can consider that approach. - from pylibcudf.interop import ColumnMetadata, _create_nested_column_metadata if metadata is None: - metadata = _create_nested_column_metadata(self) + metadata = self._create_nested_column_metadata() elif isinstance(metadata, str): metadata = ColumnMetadata(metadata) diff --git a/python/pylibcudf/pylibcudf/interop.pxd b/python/pylibcudf/pylibcudf/interop.pxd index ccbfb8e928c1..7cf3be08e09c 100644 --- a/python/pylibcudf/pylibcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/interop.pxd @@ -2,8 +2,6 @@ from pylibcudf.table cimport Table -from pylibcudf.libcudf.interop cimport column_metadata - cpdef Table from_dlpack(object managed_tensor) cpdef object to_dlpack(Table input) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index e712c987f100..0948f0120243 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -9,7 +9,6 @@ from cpython.pycapsule cimport ( from libcpp.memory cimport unique_ptr from libcpp.utility cimport move -from dataclasses import dataclass, field from functools import singledispatch from pyarrow import lib as pa @@ -26,6 +25,7 @@ from .column cimport Column from .scalar cimport Scalar from .table cimport Table from .types cimport DataType, type_id +from ._interop import ColumnMetadata __all__ = [ "ColumnMetadata", @@ -65,24 +65,6 @@ LIBCUDF_TO_ARROW_TYPES = { } -@dataclass -class ColumnMetadata: - """Metadata associated with a column. - - This is the Python representation of :cpp:class:`cudf::column_metadata`. - """ - name: str = "" - children_meta: list[ColumnMetadata] = field(default_factory=list) - - -def _create_nested_column_metadata(Column col): - return ColumnMetadata( - children_meta=[ - _create_nested_column_metadata(child) for child in col.children() - ] - ) - - @singledispatch def from_arrow(pyarrow_object, *, DataType data_type=None): """Create a cudf object from a pyarrow object. diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 5e458070897a..c809f8e4a9f0 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -22,54 +22,26 @@ from pylibcudf.libcudf.interop cimport ( ArrowSchema, arrow_table, column_metadata, - release_arrow_array_raw, - release_arrow_schema_raw, to_arrow_host_raw, to_arrow_schema_raw, ) from .column cimport Column from .utils cimport _get_stream +from pylibcudf._interop cimport ( + _release_schema, + _release_array, + _metadata_to_libcudf, +) +from ._interop import ColumnMetadata from functools import singledispatchmethod __all__ = ["Table"] -cdef void _release_schema(object schema_capsule) noexcept: - """Release the ArrowSchema object stored in a PyCapsule.""" - cdef ArrowSchema* schema = PyCapsule_GetPointer( - schema_capsule, 'arrow_schema' - ) - release_arrow_schema_raw(schema) - - -cdef void _release_array(object array_capsule) noexcept: - """Release the ArrowArray object stored in a PyCapsule.""" - cdef ArrowArray* array = PyCapsule_GetPointer( - array_capsule, 'arrow_array' - ) - release_arrow_array_raw(array) - - -cdef column_metadata _metadata_to_libcudf(metadata): - """Convert a ColumnMetadata object to C++ column_metadata. - - Since this class is mutable and cheap, it is easier to create the C++ - object on the fly rather than have it directly backing the storage for - the Cython class. Additionally, this structure restricts the dependency - on C++ types to just within this module, allowing us to make the module a - pure Python module (from an import sense, i.e. no pxd declarations). - """ - cdef column_metadata c_metadata - c_metadata.name = metadata.name.encode() - for child_meta in metadata.children_meta: - c_metadata.children_meta.push_back(_metadata_to_libcudf(child_meta)) - return c_metadata - - class _ArrowLikeMeta(type): - # Unfortunately we cannot separate stream and array via singledispatch because the + # We cannot separate stream and array via singledispatch because the # dispatch will often be ambiguous when objects expose both protocols. def __subclasscheck__(cls, other): return ( @@ -221,14 +193,9 @@ cdef class Table: def _to_schema(self, metadata=None): """Create an Arrow schema from this table.""" - # TODO: We'll need to reshuffle where things are defined to avoid circular - # imports. For now, we'll just import this inline. We should be able to avoid - # circularity altogether by simply not needing ColumnMetadata at all in the - # future and just using the schema directly, so we can consider that approach. - from pylibcudf.interop import ColumnMetadata, _create_nested_column_metadata if metadata is None: metadata = [ - _create_nested_column_metadata(col) for col in self.columns() + col._create_nested_column_metadata() for col in self.columns() ] else: metadata = [ From db92c88738dc35236426449e182382df433a4761 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 9 Apr 2025 22:53:42 +0000 Subject: [PATCH 26/41] Rename to _interop_helpers --- python/pylibcudf/pylibcudf/CMakeLists.txt | 4 ++-- .../pylibcudf/{_interop.pxd => _interop_helpers.pxd} | 0 .../pylibcudf/{_interop.pyx => _interop_helpers.pyx} | 0 python/pylibcudf/pylibcudf/column.pyx | 4 ++-- python/pylibcudf/pylibcudf/interop.pyx | 2 +- python/pylibcudf/pylibcudf/table.pyx | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) rename python/pylibcudf/pylibcudf/{_interop.pxd => _interop_helpers.pxd} (100%) rename python/pylibcudf/pylibcudf/{_interop.pyx => _interop_helpers.pyx} (100%) diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index a267ee249ebc..8ca30e5c2a02 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -28,7 +28,7 @@ set(cython_sources groupby.pyx hashing.pyx interop.pyx - _interop.pyx + _interop_helpers.pyx jit.pyx join.pyx json.pyx @@ -69,7 +69,7 @@ include(../../../cpp/cmake/thirdparty/get_nanoarrow.cmake) target_link_libraries(pylibcudf_interop PUBLIC nanoarrow) target_link_libraries(pylibcudf_table PUBLIC nanoarrow) target_link_libraries(pylibcudf_column PUBLIC nanoarrow) -target_link_libraries(pylibcudf__interop PUBLIC nanoarrow) +target_link_libraries(pylibcudf__interop_helpers PUBLIC nanoarrow) add_subdirectory(libcudf) add_subdirectory(strings) diff --git a/python/pylibcudf/pylibcudf/_interop.pxd b/python/pylibcudf/pylibcudf/_interop_helpers.pxd similarity index 100% rename from python/pylibcudf/pylibcudf/_interop.pxd rename to python/pylibcudf/pylibcudf/_interop_helpers.pxd diff --git a/python/pylibcudf/pylibcudf/_interop.pyx b/python/pylibcudf/pylibcudf/_interop_helpers.pyx similarity index 100% rename from python/pylibcudf/pylibcudf/_interop.pyx rename to python/pylibcudf/pylibcudf/_interop_helpers.pyx diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index c2258e6f55a2..0e9a484ac5c8 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -37,12 +37,12 @@ from .scalar cimport Scalar from .types cimport DataType, size_of, type_id from .utils cimport int_to_bitmask_ptr, int_to_void_ptr, _get_stream from .null_mask cimport bitmask_allocation_size_bytes -from ._interop cimport ( +from ._interop_helpers cimport ( _release_schema, _release_array, _metadata_to_libcudf, ) -from ._interop import ColumnMetadata +from ._interop_helpers import ColumnMetadata import functools diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 0948f0120243..3355a997c40d 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -25,7 +25,7 @@ from .column cimport Column from .scalar cimport Scalar from .table cimport Table from .types cimport DataType, type_id -from ._interop import ColumnMetadata +from ._interop_helpers import ColumnMetadata __all__ = [ "ColumnMetadata", diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index c809f8e4a9f0..783005ac2dd6 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -28,12 +28,12 @@ from pylibcudf.libcudf.interop cimport ( from .column cimport Column from .utils cimport _get_stream -from pylibcudf._interop cimport ( +from pylibcudf._interop_helpers cimport ( _release_schema, _release_array, _metadata_to_libcudf, ) -from ._interop import ColumnMetadata +from ._interop_helpers import ColumnMetadata from functools import singledispatchmethod From 7beecf67cb7c54ddb8ef5205777c8ce08f1b81da Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 9 Apr 2025 22:57:54 +0000 Subject: [PATCH 27/41] Some cleanup and commenting --- python/pylibcudf/pylibcudf/column.pyx | 4 +++- python/pylibcudf/pylibcudf/interop.pyx | 3 --- python/pylibcudf/pylibcudf/table.pyx | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index 0e9a484ac5c8..f2bd0badd215 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -30,7 +30,6 @@ from pylibcudf.libcudf.interop cimport ( from rmm.pylibrmm.device_buffer cimport DeviceBuffer from rmm.pylibrmm.stream cimport Stream - from .gpumemoryview cimport gpumemoryview from .filling cimport sequence from .scalar cimport Scalar @@ -60,10 +59,12 @@ class _ArrowLike(metaclass=_ArrowLikeMeta): cdef class _ArrowColumnHolder: + """A holder for an Arrow column for gpumemoryview lifetime management.""" cdef unique_ptr[arrow_column] col cdef class OwnerWithCAI: + """An interface for column view's data with gpumemoryview via CAI.""" @staticmethod cdef create(column_view cv, object owner): obj = OwnerWithCAI() @@ -106,6 +107,7 @@ cdef class OwnerWithCAI: cdef class OwnerMaskWithCAI: + """An interface for column view's null mask with gpumemoryview via CAI.""" @staticmethod cdef create(column_view cv, object owner): obj = OwnerMaskWithCAI() diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 3355a997c40d..f0f9278d47bd 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -200,9 +200,6 @@ class _ObjectWithArrowMetadata: @to_arrow.register(Table) def _to_arrow_table(cudf_object, metadata=None): """Create a PyArrow table from a pylibcudf table.""" - # TODO: See if we can stop supporting configuration of struct field names when - # exporting to arrow data. That would allow us to get rid of the - # _ObjectWithArrowMetadata struct and just use the underlying Table directly. return pa.table(_ObjectWithArrowMetadata(cudf_object, metadata)) diff --git a/python/pylibcudf/pylibcudf/table.pyx b/python/pylibcudf/pylibcudf/table.pyx index 783005ac2dd6..4992a02ab1c7 100644 --- a/python/pylibcudf/pylibcudf/table.pyx +++ b/python/pylibcudf/pylibcudf/table.pyx @@ -55,6 +55,7 @@ class _ArrowLike(metaclass=_ArrowLikeMeta): cdef class _ArrowTableHolder: + """A holder for an Arrow table for gpumemoryview lifetime management.""" cdef unique_ptr[arrow_table] tbl From 7c917a245704b5df17ec7a7fc61cd27d33fdd530 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 9 Apr 2025 23:12:39 +0000 Subject: [PATCH 28/41] Fix cudf Python chunk expectations --- python/cudf/cudf/core/column/column.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/cudf/cudf/core/column/column.py b/python/cudf/cudf/core/column/column.py index 91586e74324c..b0af1e5f784f 100644 --- a/python/cudf/cudf/core/column/column.py +++ b/python/cudf/cudf/core/column/column.py @@ -750,7 +750,7 @@ def to_arrow(self) -> pa.Array: 4 ] """ - return plc.interop.to_arrow(self.to_pylibcudf(mode="read")).chunk(0) + return plc.interop.to_arrow(self.to_pylibcudf(mode="read")) @classmethod def from_arrow(cls, array: pa.Array) -> ColumnBase: From 1e4e37e3e6455bd8c3f7a38bc6bfe8683804de43 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 10 Apr 2025 00:56:31 +0000 Subject: [PATCH 29/41] Add one more include of dlpack --- python/pylibcudf/pylibcudf/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index 8ca30e5c2a02..49b6d33e8686 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -63,6 +63,7 @@ rapids_cython_create_modules( ) target_include_directories(pylibcudf_interop PUBLIC "$") +target_include_directories(pylibcudf__interop PUBLIC "$") include(${rapids-cmake-dir}/export/find_package_root.cmake) include(../../../cpp/cmake/thirdparty/get_nanoarrow.cmake) From e26bd3c3f204db091cbf8d3613cc895cda39a730 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 10 Apr 2025 01:17:34 +0000 Subject: [PATCH 30/41] Typo --- python/pylibcudf/pylibcudf/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index 49b6d33e8686..8b32564f9bb0 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -63,7 +63,9 @@ rapids_cython_create_modules( ) target_include_directories(pylibcudf_interop PUBLIC "$") -target_include_directories(pylibcudf__interop PUBLIC "$") +target_include_directories( + pylibcudf__interop_helpers PUBLIC "$" +) include(${rapids-cmake-dir}/export/find_package_root.cmake) include(../../../cpp/cmake/thirdparty/get_nanoarrow.cmake) From a6b8284798fdb5cb72986820b58963fa43d944dc Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 10 Apr 2025 04:33:32 +0000 Subject: [PATCH 31/41] More targets --- python/pylibcudf/pylibcudf/CMakeLists.txt | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/python/pylibcudf/pylibcudf/CMakeLists.txt b/python/pylibcudf/pylibcudf/CMakeLists.txt index 8b32564f9bb0..0e96a7531fb4 100644 --- a/python/pylibcudf/pylibcudf/CMakeLists.txt +++ b/python/pylibcudf/pylibcudf/CMakeLists.txt @@ -62,17 +62,13 @@ rapids_cython_create_modules( LINKED_LIBRARIES "${linked_libraries}" MODULE_PREFIX pylibcudf_ ASSOCIATED_TARGETS cudf ) -target_include_directories(pylibcudf_interop PUBLIC "$") -target_include_directories( - pylibcudf__interop_helpers PUBLIC "$" -) - include(${rapids-cmake-dir}/export/find_package_root.cmake) include(../../../cpp/cmake/thirdparty/get_nanoarrow.cmake) -target_link_libraries(pylibcudf_interop PUBLIC nanoarrow) -target_link_libraries(pylibcudf_table PUBLIC nanoarrow) -target_link_libraries(pylibcudf_column PUBLIC nanoarrow) -target_link_libraries(pylibcudf__interop_helpers PUBLIC nanoarrow) + +foreach(source interop _interop_helpers table column) + target_include_directories(pylibcudf_${source} PUBLIC "$") + target_link_libraries(pylibcudf_${source} PUBLIC nanoarrow) +endforeach() add_subdirectory(libcudf) add_subdirectory(strings) From 83acdccaaa64f1bebef8de023dec1655f3a44cfe Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 10 Apr 2025 18:30:21 +0000 Subject: [PATCH 32/41] Fix prefetching for EMPTY --- cpp/src/column/column_view.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cpp/src/column/column_view.cpp b/cpp/src/column/column_view.cpp index a7718d19a945..d7214b6d06e6 100644 --- a/cpp/src/column/column_view.cpp +++ b/cpp/src/column/column_view.cpp @@ -37,7 +37,10 @@ template void prefetch_col_data(ColumnView& col, void const* data_ptr, std::string_view key) noexcept { if (cudf::experimental::prefetch::detail::prefetch_config::instance().get(key)) { - if (cudf::is_fixed_width(col.type())) { + if (col.type().id() == cudf::type_id::EMPTY) { + // Skip prefetching for empty columns + return; + } else if (cudf::is_fixed_width(col.type())) { cudf::experimental::prefetch::detail::prefetch_noexcept( key, data_ptr, col.size() * size_of(col.type()), cudf::get_default_stream()); } else if (col.type().id() == type_id::STRING) { From a141d21022ac30c439dbfd649f435887c428b8c7 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 10 Apr 2025 18:45:31 +0000 Subject: [PATCH 33/41] New envs need updates --- conda/environments/all_cuda-118_arch-aarch64.yaml | 2 +- conda/environments/all_cuda-128_arch-aarch64.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conda/environments/all_cuda-118_arch-aarch64.yaml b/conda/environments/all_cuda-118_arch-aarch64.yaml index 66ade735abf5..355cdb05b039 100644 --- a/conda/environments/all_cuda-118_arch-aarch64.yaml +++ b/conda/environments/all_cuda-118_arch-aarch64.yaml @@ -70,7 +70,7 @@ dependencies: - pyarrow>=14.0.0,<20.0.0a0 - pydata-sphinx-theme>=0.15.4 - pynvml>=12.0.0,<13.0.0a0 -- pytest-benchmark +- pytest-benchmark<5.1.0 - pytest-cases>=3.8.2 - pytest-cov - pytest-xdist diff --git a/conda/environments/all_cuda-128_arch-aarch64.yaml b/conda/environments/all_cuda-128_arch-aarch64.yaml index 73517abc833f..a90ce3ab39ba 100644 --- a/conda/environments/all_cuda-128_arch-aarch64.yaml +++ b/conda/environments/all_cuda-128_arch-aarch64.yaml @@ -69,7 +69,7 @@ dependencies: - pydata-sphinx-theme>=0.15.4 - pynvjitlink>=0.0.0a0 - pynvml>=12.0.0,<13.0.0a0 -- pytest-benchmark +- pytest-benchmark<5.1.0 - pytest-cases>=3.8.2 - pytest-cov - pytest-xdist From 1710d4ed55edd5abb64e5c9c2c97e4c73b17398c Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:34:14 +0000 Subject: [PATCH 34/41] Add helper function for empty columns --- cpp/src/interop/to_arrow_device.cu | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index 7b0ef1630043..6407e384644c 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -134,6 +134,14 @@ struct dispatch_to_arrow_device { } }; +int handle_empty_type_column(ArrowArray* array, cudf::column& column) +{ + NANOARROW_RETURN_NOT_OK(initialize_array(array, NANOARROW_TYPE_NA, column.view())); + auto child_contents = column.release(); + NANOARROW_RETURN_NOT_OK(set_contents(child_contents, array)); + return NANOARROW_OK; +} + template <> int dispatch_to_arrow_device::operator()(cudf::column&& column, rmm::cuda_stream_view stream, @@ -228,9 +236,7 @@ int dispatch_to_arrow_device::operator()(cudf::column&& colum ArrowArray* child_ptr = tmp->children[i]; auto& child = contents.children[i]; if (child->type().id() == cudf::type_id::EMPTY) { - NANOARROW_RETURN_NOT_OK(initialize_array(child_ptr, NANOARROW_TYPE_NA, child->view())); - auto child_contents = child->release(); - NANOARROW_RETURN_NOT_OK(set_contents(child_contents, tmp->children[i])); + NANOARROW_RETURN_NOT_OK(handle_empty(child_ptr, *child)); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, child_ptr)); @@ -260,9 +266,7 @@ int dispatch_to_arrow_device::operator()(cudf::column&& column, auto& child = contents.children[cudf::lists_column_view::child_column_index]; if (child->type().id() == cudf::type_id::EMPTY) { - NANOARROW_RETURN_NOT_OK(initialize_array(tmp->children[0], NANOARROW_TYPE_NA, child->view())); - auto child_contents = child->release(); - NANOARROW_RETURN_NOT_OK(set_contents(child_contents, tmp->children[0])); + NANOARROW_RETURN_NOT_OK(handle_empty(tmp->children[0], *child)); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, tmp->children[0])); @@ -548,9 +552,7 @@ unique_device_array_t to_arrow_device(cudf::table&& table, auto child = tmp->children[i]; auto col = cols[i].get(); if (col->type().id() == cudf::type_id::EMPTY) { - NANOARROW_THROW_NOT_OK(initialize_array(child, NANOARROW_TYPE_NA, col->view())); - auto contents = col->release(); - NANOARROW_THROW_NOT_OK(set_contents(contents, child)); + NANOARROW_THROW_NOT_OK(handle_empty_type_column(child, *col)); } else { NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( col->type(), detail::dispatch_to_arrow_device{}, std::move(*col), stream, mr, child)); @@ -567,9 +569,7 @@ unique_device_array_t to_arrow_device(cudf::column&& col, nanoarrow::UniqueArray tmp; if (col.type().id() == cudf::type_id::EMPTY) { - NANOARROW_THROW_NOT_OK(initialize_array(tmp.get(), NANOARROW_TYPE_NA, col)); - auto contents = col.release(); - NANOARROW_THROW_NOT_OK(set_contents(contents, tmp.get())); + NANOARROW_THROW_NOT_OK(handle_empty_type_column(tmp.get(), col)); } else { NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( col.type(), detail::dispatch_to_arrow_device{}, std::move(col), stream, mr, tmp.get())); From b7ee1bf9f7394bbf1ad1ff9ba60a7f1903af79eb Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:35:10 +0000 Subject: [PATCH 35/41] Add missing throw --- cpp/src/interop/to_arrow_schema.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/interop/to_arrow_schema.cpp b/cpp/src/interop/to_arrow_schema.cpp index 3b5415f02366..5fbda0dea4cf 100644 --- a/cpp/src/interop/to_arrow_schema.cpp +++ b/cpp/src/interop/to_arrow_schema.cpp @@ -227,7 +227,7 @@ unique_schema_t to_arrow_schema(cudf::table_view const& input, child->flags = col.has_nulls() ? ARROW_FLAG_NULLABLE : 0; if (col.type().id() == cudf::type_id::EMPTY) { - ArrowSchemaSetType(child, NANOARROW_TYPE_NA); + NANOARROW_THROW_NOT_OK(ArrowSchemaSetType(child, NANOARROW_TYPE_NA)); } else { NANOARROW_THROW_NOT_OK(cudf::type_dispatcher( col.type(), detail::dispatch_to_arrow_type{}, col, metadata[i], child)); From 932b0ad4035088ccfd8a1300c6fa140ad42ee13d Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:41:43 +0000 Subject: [PATCH 36/41] Switch to uintptr_t --- python/pylibcudf/pylibcudf/column.pyx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index 6543d7d2f59a..75bb53c1748b 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -7,6 +7,8 @@ from cpython.pycapsule cimport ( PyCapsule_New, ) +from libc.stdint cimport uintptr_t + from libcpp.limits cimport numeric_limits from libcpp.memory cimport make_unique, unique_ptr from libcpp.utility cimport move @@ -97,7 +99,7 @@ cdef class OwnerWithCAI: # streams of the appropriate size. This matches what we currently get from # rmm.DeviceBuffer "typestr": "|u1", - "data": ( cv.head[char](), False), + "data": ( cv.head[char](), False), "version": 3, } return obj @@ -121,7 +123,7 @@ cdef class OwnerMaskWithCAI: # streams of the appropriate size. This matches what we currently get from # rmm.DeviceBuffer "typestr": "|u1", - "data": ( cv.null_mask(), False), + "data": ( cv.null_mask(), False), "version": 3, } return obj From fd5ab31a9e5cd432ffc01f30b98878278bdb0c5e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:43:15 +0000 Subject: [PATCH 37/41] Rename cudf_object to plc_object --- python/pylibcudf/pylibcudf/interop.pyx | 32 +++++++++++++------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index f0f9278d47bd..b7b83c35586c 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -130,12 +130,12 @@ def _from_arrow_column(pyarrow_object, *, DataType data_type=None): @singledispatch -def to_arrow(cudf_object, metadata=None): +def to_arrow(plc_object, metadata=None): """Convert to a PyArrow object. Parameters ---------- - cudf_object : Union[Column, Table, Scalar] + plc_object : Union[Column, Table, Scalar] The cudf object to convert. metadata : list The metadata to attach to the columns of the table. @@ -145,11 +145,11 @@ def to_arrow(cudf_object, metadata=None): Union[pyarrow.Array, pyarrow.Table, pyarrow.Scalar] The converted object of type corresponding to the input type in PyArrow. """ - raise TypeError(f"Unsupported type {type(cudf_object)} for conversion to arrow") + raise TypeError(f"Unsupported type {type(plc_object)} for conversion to arrow") @to_arrow.register(DataType) -def _to_arrow_datatype(cudf_object, **kwargs): +def _to_arrow_datatype(plc_object, **kwargs): """ Convert a datatype to arrow. @@ -160,20 +160,20 @@ def _to_arrow_datatype(cudf_object, **kwargs): - When translating a struct type, provide ``fields`` - When translating a list type, provide the wrapped ``value_type`` """ - if cudf_object.id() in {type_id.DECIMAL32, type_id.DECIMAL64, type_id.DECIMAL128}: + if plc_object.id() in {type_id.DECIMAL32, type_id.DECIMAL64, type_id.DECIMAL128}: if not (precision := kwargs.get("precision")): raise ValueError( "Precision must be provided for decimal types" ) # no pa.decimal32 or pa.decimal64 - return pa.decimal128(precision, -cudf_object.scale()) - elif cudf_object.id() == type_id.STRUCT: + return pa.decimal128(precision, -plc_object.scale()) + elif plc_object.id() == type_id.STRUCT: if not (fields := kwargs.get("fields")): raise ValueError( "Fields must be provided for struct types" ) return pa.struct(fields) - elif cudf_object.id() == type_id.LIST: + elif plc_object.id() == type_id.LIST: if not (value_type := kwargs.get("value_type")): raise ValueError( "Value type must be provided for list types" @@ -181,10 +181,10 @@ def _to_arrow_datatype(cudf_object, **kwargs): return pa.list_(value_type) else: try: - return LIBCUDF_TO_ARROW_TYPES[cudf_object.id()] + return LIBCUDF_TO_ARROW_TYPES[plc_object.id()] except KeyError: raise TypeError( - f"Unable to convert {cudf_object.id()} to arrow datatype" + f"Unable to convert {plc_object.id()} to arrow datatype" ) @@ -198,22 +198,22 @@ class _ObjectWithArrowMetadata: @to_arrow.register(Table) -def _to_arrow_table(cudf_object, metadata=None): +def _to_arrow_table(plc_object, metadata=None): """Create a PyArrow table from a pylibcudf table.""" - return pa.table(_ObjectWithArrowMetadata(cudf_object, metadata)) + return pa.table(_ObjectWithArrowMetadata(plc_object, metadata)) @to_arrow.register(Column) -def _to_arrow_array(cudf_object, metadata=None): +def _to_arrow_array(plc_object, metadata=None): """Create a PyArrow array from a pylibcudf column.""" - return pa.array(_ObjectWithArrowMetadata(cudf_object, metadata)) + return pa.array(_ObjectWithArrowMetadata(plc_object, metadata)) @to_arrow.register(Scalar) -def _to_arrow_scalar(cudf_object, metadata=None): +def _to_arrow_scalar(plc_object, metadata=None): # Note that metadata for scalars is primarily important for preserving # information on nested types since names are otherwise irrelevant. - return to_arrow(Column.from_scalar(cudf_object, 1), metadata=metadata)[0] + return to_arrow(Column.from_scalar(plc_object, 1), metadata=metadata)[0] cpdef Table from_dlpack(object managed_tensor): From 3176f395aef11eb5c1a1c7905cc5b5bf1b829e23 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:47:22 +0000 Subject: [PATCH 38/41] Add comments on inline C++ --- python/pylibcudf/pylibcudf/libcudf/interop.pxd | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/python/pylibcudf/pylibcudf/libcudf/interop.pxd b/python/pylibcudf/pylibcudf/libcudf/interop.pxd index 8adc9827047f..ab1d5bf5d5b0 100644 --- a/python/pylibcudf/pylibcudf/libcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/libcudf/interop.pxd @@ -127,6 +127,17 @@ cdef extern from *: delete array; } """ + # The `to_*_raw` functions are all defined in the above extern block as wrappers + # around libcudf functions that return unique_ptrs with non-default deleters, which + # are nontrivial to wrap in Cython. Since we need to manage them as raw pointers in + # Cython anyway, the inline C++ functions above are the simplest way to bridge the + # gap from a language syntax perspective. + # + # The corresponding `release_*_raw` functions are needed because while the arrow + # types are pure C structs, we allocate them with new in C++ and need to use delete + # to free them. Unfortunately, unless we lie to Cython and tell it that these types + # are cppclasses, Cython will not allow the usage of the del Python keyword to + # generate the delete call, so inline C++ is again the best option. cdef ArrowSchema *to_arrow_schema_raw( const table_view& tbl, const vector[column_metadata]& metadata, From 457f67742385d80a275fa649196678ff4374aaab Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:50:51 +0000 Subject: [PATCH 39/41] Remove redundant is_fixed_point check --- python/pylibcudf/pylibcudf/column.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/pylibcudf/pylibcudf/column.pyx b/python/pylibcudf/pylibcudf/column.pyx index 75bb53c1748b..7e1fec675ce6 100644 --- a/python/pylibcudf/pylibcudf/column.pyx +++ b/python/pylibcudf/pylibcudf/column.pyx @@ -18,7 +18,7 @@ from pylibcudf.libcudf.column.column_factories cimport make_column_from_scalar from pylibcudf.libcudf.interop cimport ArrowArray, ArrowSchema, arrow_column from pylibcudf.libcudf.scalar.scalar cimport scalar, numeric_scalar from pylibcudf.libcudf.types cimport size_type, size_of as cpp_size_of, bitmask_type -from pylibcudf.libcudf.utilities.traits cimport is_fixed_width, is_fixed_point +from pylibcudf.libcudf.utilities.traits cimport is_fixed_width from pylibcudf.libcudf.copying cimport get_element from pylibcudf.libcudf.interop cimport ( @@ -77,7 +77,7 @@ cdef class OwnerWithCAI: cdef unique_ptr[scalar] last_offset if cv.type().id() == type_id.EMPTY: size = cv.size() - elif is_fixed_width(cv.type()) or is_fixed_point(cv.type()): + elif is_fixed_width(cv.type()): size = cv.size() * cpp_size_of(cv.type()) elif cv.type().id() == type_id.STRING: # The size of the character array in the parent is the offsets size From bb8a2f052b29fff1c14a8e18b5af17be8784d032 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 21:55:31 +0000 Subject: [PATCH 40/41] Minor cleanup --- cpp/src/interop/from_arrow_device.cu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/interop/from_arrow_device.cu b/cpp/src/interop/from_arrow_device.cu index 6d634ade3b0a..74eb6d1e6b29 100644 --- a/cpp/src/interop/from_arrow_device.cu +++ b/cpp/src/interop/from_arrow_device.cu @@ -284,7 +284,7 @@ dispatch_tuple_t dispatch_from_arrow_device::operator()( size_type const offset = input->offset; size_type const null_count = input->null_count; auto offsets_view = column_view{data_type(type_id::INT32), - num_rows == 0 ? 0 : offset + num_rows + 1, + (num_rows == 0) ? 0 : (offset + num_rows + 1), input->buffers[fixed_width_data_buffer_idx], nullptr, 0, From 1a3197cd1d3dfc6cd601359369c4db28ea73ae8a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 15 Apr 2025 22:20:58 +0000 Subject: [PATCH 41/41] Fix incomplete replacement --- cpp/src/interop/to_arrow_device.cu | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index 6407e384644c..1f043420daf8 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -236,7 +236,7 @@ int dispatch_to_arrow_device::operator()(cudf::column&& colum ArrowArray* child_ptr = tmp->children[i]; auto& child = contents.children[i]; if (child->type().id() == cudf::type_id::EMPTY) { - NANOARROW_RETURN_NOT_OK(handle_empty(child_ptr, *child)); + NANOARROW_RETURN_NOT_OK(handle_empty_type_column(child_ptr, *child)); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, child_ptr)); @@ -266,7 +266,7 @@ int dispatch_to_arrow_device::operator()(cudf::column&& column, auto& child = contents.children[cudf::lists_column_view::child_column_index]; if (child->type().id() == cudf::type_id::EMPTY) { - NANOARROW_RETURN_NOT_OK(handle_empty(tmp->children[0], *child)); + NANOARROW_RETURN_NOT_OK(handle_empty_type_column(tmp->children[0], *child)); } else { NANOARROW_RETURN_NOT_OK(cudf::type_dispatcher( child->type(), dispatch_to_arrow_device{}, std::move(*child), stream, mr, tmp->children[0]));