ARROW-12060: [Python] Enable calling compute functions on Expressions #11918
Conversation
|
|
|
This makes the following already possible (to use in filter or projection expression in dataset): |
9b8a103 to
3b4b0fc
Compare
python/pyarrow/_compute.pyx
Outdated
| c_arguments.push_back((<Expression> argument).expr) | ||
|
|
||
| # if options is not None: | ||
| # c_options = make_shared[CFunctionOptions](options.get_options()) |
There was a problem hiding this comment.
Given a python FunctionOptions class object, I need to get a shared pointer to the C++ object (get_options() returns a const CFunctionOptions*, but CmakeCallExpressions needs a shared_ptr[CFunctionOptions]).
But I haven't directly found how to do that conversion (cc @pitrou)
There was a problem hiding this comment.
You should probably replace the unique_ptr in the FunctionOptions declaration with a shared_ptr.
There was a problem hiding this comment.
The DeserializeFunctionOptions in C++ (in function_internal.h, and several other methods there) returns a unique_ptr(from #10511). Can that be changed there as well?
There was a problem hiding this comment.
You can, but I don't think it's necessary on the C++ side. On the Python side, you can convert the unique_ptr into a shared_ptr (by moving it).
There was a problem hiding this comment.
Yes, that's what I did in the meantime. The only difficulty I ran into is that moving a uniqe_ptr to shared_ptr doesn't seem to work in cython, but with our helper to_shared it worked.
python/pyarrow/_compute.pyx
Outdated
| cdef: | ||
| vector[CExpression] c_arguments | ||
| shared_ptr[CFunctionOptions] c_options=( | ||
| <shared_ptr[CFunctionOptions]> nullptr) |
There was a problem hiding this comment.
You don't need to explicitly initialize this, do you?
There was a problem hiding this comment.
Ah, yes, passing an unitialized c_options to CMakeCallExpression seems to work as well
|
Benchmark runs are scheduled for baseline = 18bedd3 and contender = 17bf54a. 17bf54a is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
No description provided.