[GitHub] [arrow] cyb70289 commented on a change in pull request #10686: ARROW-13289: [C++] Accept integer args in trig/log functions via promotion to double

[GitBox <gi...@apache.org>]

cyb70289 commented on a change in pull request #10686:
URL: https://github.com/apache/arrow/pull/10686#discussion_r667592918



##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1042,6 +1042,26 @@ TEST(TestUnaryArithmetic, DispatchBest) {
   for (std::string name : {"negate", "negate_checked", "abs", "abs_checked"}) {
     CheckDispatchFails(name, {null()});
   }
+
+  for (std::string name :
+       {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos"}) {
+    for (std::string suffix : {"", "_checked"}) {
+      name += suffix;
+
+      CheckDispatchBest(name, {int32()}, {float64()});
+      CheckDispatchBest(name, {uint8()}, {float64()});
+
+      CheckDispatchBest(name, {dictionary(int8(), int64())}, {float64()});
+    }
+  }
+
+  CheckDispatchBest("atan", {int32()}, {float64()});
+  CheckDispatchBest("atan2", {int32(), float64()}, {float64(), float64()});
+  CheckDispatchBest("atan2", {int32(), uint8()}, {float64(), float64()});
+  CheckDispatchBest("atan2", {int32(), null()}, {float64(), float64()});
+  CheckDispatchBest("atan2", {float32(), float64()}, {float64(), float64()});
+  // Integer always promotes to double
+  CheckDispatchBest("atan2", {float32(), int8()}, {float64(), float64()});

Review comment:
       Is it necessary to add some integer tests besides the implicit cast?

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -1076,6 +1076,38 @@ struct ArithmeticFunction : ScalarFunction {
   }
 };
 
+/// An ArithmeticFunction that promotes integer arguments to double.
+struct ArithmeticFloatingPointFunction : public ArithmeticFunction {
+  using ArithmeticFunction::ArithmeticFunction;
+
+  Result<const Kernel*> DispatchBest(std::vector<ValueDescr>* values) const override {
+    RETURN_NOT_OK(CheckArity(*values));
+    RETURN_NOT_OK(CheckDecimals(values));
+
+    using arrow::compute::detail::DispatchExactImpl;
+    if (auto kernel = DispatchExactImpl(this, *values)) return kernel;
+
+    EnsureDictionaryDecoded(values);
+
+    // Only promote types for binary functions

Review comment:
       This comment looks not correct.




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org