From f655602ecd8f254dfcee5eb0c790bd3336e83d7c Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Tue, 11 Jun 2024 15:59:20 -0700 Subject: [PATCH] Fix Cython typo preventing proper inheritance (#15978) #15831 added new inheritance patterns to the Parquet options classes, but mirroring them perfectly in Cython proved problematic due to what appeared to be issues with Cython parsing of CRTP and inheritance. A deeper investigation revealed that the underlying issue was https://github.com/cython/cython/issues/6238. This PR applies the appropriate fix. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Thomas Li (https://github.com/lithomas1) - Bradley Dice (https://github.com/bdice) URL: https://github.com/rapidsai/cudf/pull/15978 --- .../_lib/pylibcudf/libcudf/io/parquet.pxd | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/python/cudf/cudf/_lib/pylibcudf/libcudf/io/parquet.pxd b/python/cudf/cudf/_lib/pylibcudf/libcudf/io/parquet.pxd index 36654457995..0ef6553db56 100644 --- a/python/cudf/cudf/_lib/pylibcudf/libcudf/io/parquet.pxd +++ b/python/cudf/cudf/_lib/pylibcudf/libcudf/io/parquet.pxd @@ -123,7 +123,7 @@ cdef extern from "cudf/io/parquet.hpp" namespace "cudf::io" nogil: ) except + cdef cppclass parquet_writer_options_builder_base[BuilderT, OptionsT]: - parquet_writer_options_builder() except + + parquet_writer_options_builder_base() except + BuilderT& metadata( cudf_io_types.table_input_metadata m @@ -164,22 +164,6 @@ cdef extern from "cudf/io/parquet.hpp" namespace "cudf::io" nogil: BuilderT& dictionary_policy( cudf_io_types.dictionary_policy val ) except + - # FIXME: the following two functions actually belong in - # parquet_writer_options_builder, but placing them there yields a - # "'parquet_writer_options_builder' is not a type identifier" error. - # This is probably a bug in cython since a simpler CRTP example that - # has methods returning references to a child class seem to work. - # Calling these from the chunked options builder will fail at compile - # time, so this should be safe. - # NOTE: these two are never actually called from libcudf. Instead these - # properties are set in the options after calling build(), so perhaps - # they can be removed. - BuilderT& partitions( - vector[cudf_io_types.partition_info] partitions - ) except + - BuilderT& column_chunks_file_paths( - vector[string] column_chunks_file_paths - ) except + OptionsT build() except + cdef cppclass parquet_writer_options_builder( @@ -190,6 +174,12 @@ cdef extern from "cudf/io/parquet.hpp" namespace "cudf::io" nogil: cudf_io_types.sink_info sink_, cudf_table_view.table_view table_ ) except + + parquet_writer_options_builder& partitions( + vector[cudf_io_types.partition_info] partitions + ) except + + parquet_writer_options_builder& column_chunks_file_paths( + vector[string] column_chunks_file_paths + ) except + cdef unique_ptr[vector[uint8_t]] write_parquet( parquet_writer_options args