Start migrating I/O to pylibcudf #15899
Conversation
lithomas1
added
feature request
github-actions
bot
added
Python
| self.c_obj = move(source_info(c_files)) | ||
| return | ||
|
|
||
| # TODO: host_buffer is deprecated API, use host_span instead |
There was a problem hiding this comment.
The deprecated API usage was originally in make_source_info, which this code is based off of.
I tried to have a go at switching over to host_span, but my C++ isn't good enough for that yet 😭 .
IMO, it should be fine to keep it as is for now, and tackle the deprecated API usage in a followup (I'll open a followup issue).
There was a problem hiding this comment.
Probably (sketch):
Add the follow declaration to utilities/host_span.pxd:
host_span(T*, size_type)
and then
from ...libcudf.utilities.host_span cimport host_span
cdef vector[host_span[char]] c_host_buffers
# fill host buffers
...
c_host_buffers.push_back(host_span[char](<char *>&c_buffer[0], c_buffer.shape[0]))
cdef host_span[host_span[char]] c_spans = host_span[host_span[char]](c_host_buffers)
...
| plc.io.SourceInfo([source]) | ||
|
|
||
| # TODO: test contents of source_info buffer is correct | ||
| # once buffers are exposed on python side |
There was a problem hiding this comment.
Accessing the buffers of the SourceInfo is a bit annoying right now, due to the host_buffer deprecation, and because the host_span bindings are incomplete.
This is probably better done after the host_buffer -> host_span deprecation adjustment.
|
|
||
| cdef avro_reader_options avro_opts = move( | ||
| avro_reader_options.builder(source_info.c_obj) | ||
| .columns(c_columns) |
There was a problem hiding this comment.
nit:
I would perhaps rather rely on the default-constructed avro_reader_options object, and not repeat default values in this interface.
We could take optional skip_rows and num_rows and do:
cdef avro_reader_options opts = move(avro_reader_options.builder(source_info.c_obj))
if skip_rows is not None:
opts = move(opts.skip_rows(skip_rows))
etc...
WDYT?
There was a problem hiding this comment.
I think the old Avro (and I/O) Cython files do it my way (passing all arguments) in general.
Unless there's a C++ trap I'm missing, I think I'd prefer to keep it more compact like this.
There was a problem hiding this comment.
I don't mind doing it "all at once", it's just that (the old code did this too), with this mechanism we encode the "default" values in two places: once canonically in libcudf C++ "skip_rows = -1 means all of them", and once non-canonically here.
|
/merge |
Description
xref #15162
Starts migrating cudf I/O cython to use pylibcudf APIs, starting with avro.
Checklist