Rewrite remaining Python Arrow interop conversions using the C Data Interface

[vyasr]

Description

This PR rewrites all remaining parts of the Python interop code previously using Arrow C++ types to instead use the C Data Interface. With this change, we no longer require pyarrow in that part of the Cython code. There are further improvements that we should make to streamline the internals, but I would like to keep this changeset minimal since getting it merged unblocks progress on multiple fronts so that we can progress further in parallel.

Contributes to #15193

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[vyasr]

@zeroshade nice work on to_arrow_host! Everything looks good here, just one small bug with nested list types that would have required a somewhat convoluted test case to catch.

[bdice]

Things seem fine to me, I don't have any significant comments. The complexity of this stuff is so high and there's a lot of "raw" objects like PyCapsule and void* everywhere... but I assume that's just the cost we have to pay for dealing with a C data interface.

[zeroshade]

@zeroshade nice work on to_arrow_host! Everything looks good here, just one small bug with nested list types that would have required a somewhat convoluted test case to catch.

Thanks! Could we add that "somewhat convoluted test case" to the unit tests so that it doesn't get missed in future changes? 😄

[vyasr]

@zeroshade @paleolimbot I believe that this PR has uncovered a bug in nanoarrow. I raised apache/arrow-nanoarrow#587 to discuss that, but let me know if anything seems wrong here in how we've implemented things in libcudf. The failing test can be run with python -m pytest python/cudf/cudf/pylibcudf_tests/test_datetime.py::test_extract_year.

[vyasr]

Based on @paleolimbot's comment here, perhaps what we should be doing is adding DATE32 and DATE64 to this switch statement? I assumed that @zeroshade had a reason for putting TIMESTAMP_DAYS where it is, was that just a typo perhaps Matt? Or maybe in that case it should be mapping to a different storage type, not in64?

[paleolimbot]

I think so! (Also will be fixed by apache/arrow-nanoarrow#588, which basically adds the same switch statement to nanoarrow so that you and/or future users don't have to deal with that 🙂 )

[vyasr]

It seems like we need to map TIMESTAMP_DAYS to NANOARROW_TYPE_INT32 as a storage type in id_to_arrow_storage_type, but we also need to keep the id_to_arrow_type mapping for TIMESTAMP_DAYS because to_arrow_schema relies on that. It seems strange to keep both around, but it doesn't seem incorrect to me. @zeroshade let me know if that strikes you as something we should streamline.

That said, adding the mapping does appear sufficient to resolve the issue.

[zeroshade]

Based on @paleolimbot's comment apache/arrow-nanoarrow#587 (comment), perhaps what we should be doing is adding DATE32 and DATE64 to this switch statement? I assumed that @zeroshade had a reason for putting TIMESTAMP_DAYS where it is, was that just a typo perhaps Matt? Or maybe in that case it should be mapping to a different storage type, not in64?

Not a typo, as you surmised we need to map TIMESTAMP_DAYS -> DATE32 for schema purposes, but to INT32 for the storage type of Array.

If you can think of a way to streamline it, then that would be awesome, but otherwise this sounds like the right solution until we can incorporate just calling it on the nanoarrow side via the PR that @paleolimbot linked to

[vyasr]

/merge