lidavidm commented on a change in pull request #10727:
URL: https://github.com/apache/arrow/pull/10727#discussion_r671366668



##########
File path: docs/source/cpp/compute.rst
##########
@@ -348,6 +348,22 @@ Bit-wise functions
   out of bounds for the data type.  However, an overflow when shifting the
   first input is not error (truncated bits are silently discarded).
 
+Rounding functions
+~~~~~~~~~~~~~~~~~~
+
+Rounding functions convert a numeric input into an approximate value with a
+simpler representation based on the rounding strategy.

Review comment:
       This is worded a little confusingly in my opinion. If we're going to 
reference rounding strategy here, the notes column should describe the rounding 
behavior for each function (even if it's just the 'obvious' or 'expected' one).
   
   Or alternatively, something like 'rounding functions find the nearest 
integer (as a floating-point value) to the argument based on a rounding 
strategy'.

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1032,47 +1032,90 @@ TEST(TestBinaryArithmetic, 
AddWithImplicitCastsUint64EdgeCase) {
 }
 
 TEST(TestUnaryArithmetic, DispatchBest) {
-  // All arithmetic
-  for (std::string name : {"negate", "abs", "abs_checked", "sign"}) {
+  // All types (with _checked variant)
+  for (std::string name : {"abs"}) {
+    for (std::string suffix : {"", "_checked"}) {
+      name += suffix;
+      for (const auto& ty : {int8(), int16(), int32(), int64(), uint8(), 
uint16(),
+                             uint32(), uint64(), float32(), float64()}) {
+        CheckDispatchBest(name, {ty}, {ty});
+        CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
+      }
+    }
+  }
+
+  // All types
+  for (std::string name : {"negate", "sign"}) {
     for (const auto& ty : {int8(), int16(), int32(), int64(), uint8(), 
uint16(), uint32(),
                            uint64(), float32(), float64()}) {
       CheckDispatchBest(name, {ty}, {ty});
       CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
     }
   }
 
-  // Signed arithmetic
+  // Fail on null type (with _checked variant)
+  for (std::string name : {"negate", "abs", "ln", "log2", "log10", "log1p", 
"sin", "cos",
+                           "tan", "asin", "acos"}) {
+    for (std::string suffix : {"", "_checked"}) {
+      name += suffix;
+      CheckDispatchFails(name, {null()});
+    }
+  }
+
+  // Fail on null type
+  for (std::string name : {"atan", "sign", "floor", "ceil", "trunc"}) {
+    CheckDispatchFails(name, {null()});
+  }
+
+  // Signed types
   for (std::string name : {"negate_checked"}) {
     for (const auto& ty : {int8(), int16(), int32(), int64(), float32(), 
float64()}) {
       CheckDispatchBest(name, {ty}, {ty});
       CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
     }
   }
 
-  // Null input
-  for (std::string name : {"negate", "negate_checked", "abs", "abs_checked", 
"sign"}) {
-    CheckDispatchFails(name, {null()});
-  }
-
+  // Float types (with _checked variant)
   for (std::string name :
        {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos"}) {
     for (std::string suffix : {"", "_checked"}) {
       name += suffix;
+      for (const auto& ty : {float32(), float64()}) {
+        CheckDispatchBest(name, {ty}, {ty});
+        CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
+      }
+    }
+  }
 
-      CheckDispatchBest(name, {int32()}, {float64()});
-      CheckDispatchBest(name, {uint8()}, {float64()});
+  // Float types
+  for (std::string name : {"atan", "floor", "ceil", "trunc"}) {
+    for (const auto& ty : {float32(), float64()}) {
+      CheckDispatchBest(name, {ty}, {ty});
+      CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
+    }
+  }
 
-      CheckDispatchBest(name, {dictionary(int8(), int64())}, {float64()});
+  // Integer -> Float64 (with _checked variant)
+  for (std::string name :
+       {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos"}) {
+    for (std::string suffix : {"", "_checked"}) {
+      name += suffix;
+      for (const auto& ty :
+           {int8(), int16(), int32(), int64(), uint8(), uint16(), uint32(), 
uint64()}) {
+        CheckDispatchBest(name, {ty}, {float64()});
+        CheckDispatchBest(name, {dictionary(int8(), ty)}, {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()});
+  // Integer -> Float64
+  for (std::string name : {"atan", "floor", "ceil", "trunc"}) {
+    for (const auto& ty :
+         {int8(), int16(), int32(), int64(), uint8(), uint16(), uint32(), 
uint64()}) {
+      CheckDispatchBest(name, {ty}, {float64()});
+      CheckDispatchBest(name, {dictionary(int8(), ty)}, {float64()});
+    }
+  }

Review comment:
       This drops the tests for atan2?




-- 
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