zanmato1984 commented on PR #48267:
URL: https://github.com/apache/arrow/pull/48267#issuecomment-3609790283

   > It looks like Python bindings got fixed by adding [option class names to 
FunctionDoc in 
vector_swizzle.cc](https://github.com/apache/arrow/pull/48267/commits/47e3e8cf8004843764b6f9fe9386ea5a1d5bbc5f#diff-82f49131a414c3c37f943cdc4b3735a875c31b420b3c5e256de63defe9773b4eL335).
   
   Nice catch! I forgot to declare the option type in function doc when I was 
adding these functions, and didn't realize it affects python binding until you 
fixed it. Really nit finding, thank you!
   
   > If `output_type` is NULLPTR in C++ (which happens if `output_type` is 
`None` in Python) then `InversePermutationOptions` can't be serialized. Any 
advice on the approach for fixing this? (Should Python users always specify 
`output_type`, or perhaps `output_type` be optional in C++? Or is it fine to 
leave it as it is?)
   
   I want to suggest the following changes to allow `std::shared_ptr<DataType>` 
being `null`, when doing function options serde:
   
https://github.com/apache/arrow/blob/e90bacded2ae5a34be11c764e00751c18ab3b355/cpp/src/arrow/compute/function_internal.h#L347-L353
   
   ```diff
   static inline Result<std::shared_ptr<Scalar>> GenericToScalar(
       const std::shared_ptr<DataType>& value) {
     if (!value) {
   -    return Status::Invalid("shared_ptr<DataType> is nullptr");
   +    return std::make_shared<NullScalar>();
     }
     return MakeNullScalar(value);
   }
   ```
   and
   
https://github.com/apache/arrow/blob/e90bacded2ae5a34be11c764e00751c18ab3b355/cpp/src/arrow/compute/function_internal.h#L448-L452
   
   ```diff
   template <typename T>
   static inline enable_if_same_result<T, std::shared_ptr<DataType>> 
GenericFromScalar(
       const std::shared_ptr<Scalar>& value) {
   +  if (value->type->id() == Type::NA) {
   +    return std::shared_ptr<NullType>();
   +  }
     return value->type;
   }
   ```
   
   But I'll need to consult @pitrou if this is appropriate.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to