Skip to content

Commit

Permalink
Fix Cython typo preventing proper inheritance (#15978)
Browse files Browse the repository at this point in the history
#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 cython/cython#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: #15978
  • Loading branch information
vyasr authored Jun 11, 2024
1 parent dfa79d4 commit f655602
Showing 1 changed file with 7 additions and 17 deletions.
24 changes: 7 additions & 17 deletions python/cudf/cudf/_lib/pylibcudf/libcudf/io/parquet.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down

0 comments on commit f655602

Please sign in to comment.