Relax Arrow pin

[vyasr]

Description

With this change, cudf users can install any version of pyarrow greater than 14. This is the minimum version supporting the C Data Interface, which is a requirement for us (it may be possible to relax in principle, but would require changes to the cudf/pylibcudf code). A few tests are skipped due to bugs or missing features in older versions of pyarrow.

Checklist

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

[galipremsagar]

In my local testing I found that the minimum required version might be 15.0.0 because __arrow_c_stream__ seems to be added in 15.0.0:

apache/arrow#38717

But it shows up in changelog of 16.0.0 only(and not 15.0.0):

However, if I install a 14.0.0 version of pyarrow, I still have __arrow_c_stream__ - It might have been possibly back-ported. is that the case? I'm not able to find history of this being back-ported.

These changes look good. But should be rather keep an upper bound pin on the pyarrow version because of the same reasons we do for other libraries?

Locally verified cudf & dask-cudf work quite well for all of the following versions (barring the failures that you already covered with pytest skips in this PR):

• 14.0.0
• 15.0.0
• 15.0.1
• 16.0.0
• 16.1.0
• 17.0.0

[vyasr]

The original capsule interface was added in apache/arrow#37797. apache/arrow#39455 looks to be specifically providing the stream capsule for chunked arrays. I believe that we use the __arrow_c_array__ protocol for those instead and rely on combining chunks, hence why Arrow 14 seems to be sufficient. Let me double check what's happening on 14 to be sure.

[vyasr]

Well, the answer is not quite as good as I had hoped. I was right that our current conversion function for ChunkedArray tries to use the __arrow_c_array__ attribute, but that doesn't actually exist so it will just error out. I do appear to be correct that we are always combining chunks as needed before attempting such a conversion though because I was able to run the entire test suite with pylibcudf modified to raise an exception if we ever pass a ChunkedArray to from_arrow. Therefore, I propose that we leave the pinning on this PR as is (allow arrow 14), but remove the erroneous overload of from_arrow for ChunkedArray. If we choose to implement it correctly in the future, we can either bump the minimum pyarrow version or gate just that bit of functionality on the right attributes existing. Does that sound OK?

[galipremsagar]

Well, the answer is not quite as good as I had hoped. I was right that our current conversion function for ChunkedArray tries to use the __arrow_c_array__ attribute, but that doesn't actually exist so it will just error out. I do appear to be correct that we are always combining chunks as needed before attempting such a conversion though because I was able to run the entire test suite with pylibcudf modified to raise an exception if we ever pass a ChunkedArray to from_arrow. Therefore, I propose that we leave the pinning on this PR as is (allow arrow 14), but remove the erroneous overload of from_arrow for ChunkedArray. If we choose to implement it correctly in the future, we can either bump the minimum pyarrow version or gate just that bit of functionality on the right attributes existing. Does that sound OK?

Yup, sounds good to me. 👍

[vyasr]

Cool. Added upper bounds as well.

[galipremsagar]

This looks good to go! Thanks @vyasr !

[jameslamb]

Just one comment, but marking this "approve" so you don't need to wait on another review from me after addressing that. @galipremsagar 's testing in #16681 (review) and the discussion after that makes me very confident in this change.

[vyasr]

For the record in case it helps inspire future confidence, independently of Prem I also ran the test suite on all the Python versions that he listed with similar results (which is what revealed the skips that I added to the test suite).

[vyasr]

/merge