tadeja commented on code in PR #48914:
URL: https://github.com/apache/arrow/pull/48914#discussion_r3913444610
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc:
##########
@@ -938,6 +938,173 @@ TYPED_TEST(TestBinaryArithmeticSigned,
DivideOverflowRaises) {
this->AssertBinop(Divide, MakeArray(min), MakeArray(-1), "[0]");
}
+// ============== REMAINDER (Truncated) Tests ==============
+
+TYPED_TEST(TestBinaryArithmeticIntegral, Remainder) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertBinop(Remainder, "[]", "[]", "[]");
+ // Basic positive cases
+ this->AssertBinop(Remainder, "[7, 10, 20]", "[3, 4, 7]", "[1, 2, 6]");
+ // Array with nulls
+ this->AssertBinop(Remainder, "[null, 10, 30, null, 20]", "[1, 4, 2, 5,
10]",
+ "[null, 2, 0, null, 0]");
+ // Scalar % Array
+ this->AssertBinop(Remainder, 33, "[null, 1, 3, null, 2]", "[null, 0, 0,
null, 1]");
+ // Array % Scalar
+ this->AssertBinop(Remainder, "[null, 10, 30, null, 2]", 3, "[null, 1, 0,
null, 2]");
+ // Scalar % Scalar
+ this->AssertBinop(Remainder, 16, 7, 2);
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, Remainder) {
+ // Truncated semantics: sign follows dividend
+ this->AssertBinop(Remainder, "[7]", "[3]", "[1]");
+ this->AssertBinop(Remainder, "[-7]", "[3]", "[-1]");
+ this->AssertBinop(Remainder, "[7]", "[-3]", "[1]");
+ this->AssertBinop(Remainder, "[-7]", "[-3]", "[-1]");
+ // Mixed array
+ this->AssertBinop(Remainder, "[-3, 2, -7, 10]", "[1, 1, 2, 3]", "[0, 0, -1,
1]");
+}
+
+TYPED_TEST(TestBinaryArithmeticUnsigned, Remainder) {
+ this->AssertBinop(Remainder, "[7, 100, 255]", "[3, 30, 16]", "[1, 10, 15]");
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, RemainderOverflow) {
+ using CType = typename TestFixture::CType;
+ auto min = std::numeric_limits<CType>::lowest();
+
+ // Unchecked: returns 0 (the mathematically correct result)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, MakeArray(min), MakeArray(CType(-1)), "[0]");
+
+ // Checked: raises overflow error
+ this->SetOverflowCheck(true);
+ this->AssertBinopRaises(Remainder, MakeArray(min), MakeArray(CType(-1)),
"overflow");
+}
+
+TYPED_TEST(TestBinaryArithmeticIntegral, RemainderByZero) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ this->AssertBinopRaises(Remainder, "[3, 2, 6]", "[1, 1, 0]", "divide by
zero");
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticFloating, Remainder) {
+ SKIP_IF_HALF_FLOAT();
+
+ this->SetNansEqual(true);
+
+ // Basic cases
+ this->AssertBinop(Remainder, "[7.5, 10.0]", "[2.5, 3.0]", "[0.0, 1.0]");
+ // Negative numbers - truncated semantics: sign follows dividend
+ this->AssertBinop(Remainder, "[-7.5]", "[2.5]", "[-0.0]");
+ this->AssertBinop(Remainder, "[7.5]", "[-2.5]", "[0.0]");
+ this->AssertBinop(Remainder, "[-7.5]", "[-2.5]", "[-0.0]");
+
+ // Division by zero returns NaN (unchecked)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, "[1.0]", "[0.0]", "[NaN]");
+
+ // Division by zero raises error (checked)
+ this->SetOverflowCheck(true);
+ this->AssertBinopRaises(Remainder, "[1.0]", "[0.0]", "divide by zero");
+
+ // Infinity edge cases (unchecked)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, "[Inf]", "[2.0]", "[NaN]");
+ this->AssertBinop(Remainder, "[-Inf]", "[2.0]", "[NaN]");
+ this->AssertBinop(Remainder, "[2.0]", "[Inf]", "[2.0]");
+ this->AssertBinop(Remainder, "[2.0]", "[-Inf]", "[2.0]");
+ this->AssertBinop(Remainder, "[Inf]", "[Inf]", "[NaN]");
+}
+
+// ============== MOD (Floored) Tests ==============
Review Comment:
ultra nitpick: MOD -> MODULO
##########
docs/source/cpp/compute.rst:
##########
@@ -566,6 +575,18 @@ Mixed time resolution temporal inputs will be cast to
finest input resolution.
intermediate stages of the computation. If either argument is infinite, the
result is ``+Inf`` even if the other argument is NaN.
+* \(4) Computes the floored modulo, where the result has the same sign as the
+ divisor. This is equivalent to Python's ``%`` operator. Integer and decimal
+ division by zero returns an error, while floating-point division by zero
+ returns NaN. Decimal arguments are promoted to a common scale ``s``; the
+ result then has ``scale = s`` and ``precision = max(p1, p2)``.
+
+* \(5) Computes the truncated remainder, where the result has the same sign as
+ the dividend. This is equivalent to C/C++'s ``%`` operator. Integer and
+ decimal division by zero returns an error, while floating-point division by
Review Comment:
... similarly:
```rst
For floating-point inputs, it returns ``NaN`` in ``remainder`` and an error
in ``remainder_checked``.
```
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc:
##########
@@ -938,6 +938,173 @@ TYPED_TEST(TestBinaryArithmeticSigned,
DivideOverflowRaises) {
this->AssertBinop(Divide, MakeArray(min), MakeArray(-1), "[0]");
}
+// ============== REMAINDER (Truncated) Tests ==============
+
+TYPED_TEST(TestBinaryArithmeticIntegral, Remainder) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertBinop(Remainder, "[]", "[]", "[]");
+ // Basic positive cases
+ this->AssertBinop(Remainder, "[7, 10, 20]", "[3, 4, 7]", "[1, 2, 6]");
+ // Array with nulls
+ this->AssertBinop(Remainder, "[null, 10, 30, null, 20]", "[1, 4, 2, 5,
10]",
+ "[null, 2, 0, null, 0]");
+ // Scalar % Array
+ this->AssertBinop(Remainder, 33, "[null, 1, 3, null, 2]", "[null, 0, 0,
null, 1]");
+ // Array % Scalar
+ this->AssertBinop(Remainder, "[null, 10, 30, null, 2]", 3, "[null, 1, 0,
null, 2]");
+ // Scalar % Scalar
+ this->AssertBinop(Remainder, 16, 7, 2);
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, Remainder) {
+ // Truncated semantics: sign follows dividend
+ this->AssertBinop(Remainder, "[7]", "[3]", "[1]");
+ this->AssertBinop(Remainder, "[-7]", "[3]", "[-1]");
+ this->AssertBinop(Remainder, "[7]", "[-3]", "[1]");
+ this->AssertBinop(Remainder, "[-7]", "[-3]", "[-1]");
+ // Mixed array
+ this->AssertBinop(Remainder, "[-3, 2, -7, 10]", "[1, 1, 2, 3]", "[0, 0, -1,
1]");
+}
+
+TYPED_TEST(TestBinaryArithmeticUnsigned, Remainder) {
+ this->AssertBinop(Remainder, "[7, 100, 255]", "[3, 30, 16]", "[1, 10, 15]");
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, RemainderOverflow) {
+ using CType = typename TestFixture::CType;
+ auto min = std::numeric_limits<CType>::lowest();
+
+ // Unchecked: returns 0 (the mathematically correct result)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, MakeArray(min), MakeArray(CType(-1)), "[0]");
+
+ // Checked: raises overflow error
+ this->SetOverflowCheck(true);
+ this->AssertBinopRaises(Remainder, MakeArray(min), MakeArray(CType(-1)),
"overflow");
+}
+
+TYPED_TEST(TestBinaryArithmeticIntegral, RemainderByZero) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ this->AssertBinopRaises(Remainder, "[3, 2, 6]", "[1, 1, 0]", "divide by
zero");
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticFloating, Remainder) {
+ SKIP_IF_HALF_FLOAT();
+
+ this->SetNansEqual(true);
+
+ // Basic cases
+ this->AssertBinop(Remainder, "[7.5, 10.0]", "[2.5, 3.0]", "[0.0, 1.0]");
+ // Negative numbers - truncated semantics: sign follows dividend
+ this->AssertBinop(Remainder, "[-7.5]", "[2.5]", "[-0.0]");
+ this->AssertBinop(Remainder, "[7.5]", "[-2.5]", "[0.0]");
+ this->AssertBinop(Remainder, "[-7.5]", "[-2.5]", "[-0.0]");
+
+ // Division by zero returns NaN (unchecked)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, "[1.0]", "[0.0]", "[NaN]");
+
+ // Division by zero raises error (checked)
+ this->SetOverflowCheck(true);
+ this->AssertBinopRaises(Remainder, "[1.0]", "[0.0]", "divide by zero");
+
+ // Infinity edge cases (unchecked)
+ this->SetOverflowCheck(false);
+ this->AssertBinop(Remainder, "[Inf]", "[2.0]", "[NaN]");
+ this->AssertBinop(Remainder, "[-Inf]", "[2.0]", "[NaN]");
+ this->AssertBinop(Remainder, "[2.0]", "[Inf]", "[2.0]");
+ this->AssertBinop(Remainder, "[2.0]", "[-Inf]", "[2.0]");
+ this->AssertBinop(Remainder, "[Inf]", "[Inf]", "[NaN]");
+}
+
+// ============== MOD (Floored) Tests ==============
+
+TYPED_TEST(TestBinaryArithmeticIntegral, Modulo) {
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertBinop(Modulo, "[]", "[]", "[]");
+ // Basic positive cases (same as remainder for positive numbers)
+ this->AssertBinop(Modulo, "[7, 10, 20]", "[3, 4, 7]", "[1, 2, 6]");
+ // Array with nulls
+ this->AssertBinop(Modulo, "[null, 10, 30, null, 20]", "[1, 4, 2, 5, 10]",
+ "[null, 2, 0, null, 0]");
+ // Scalar % Array
+ this->AssertBinop(Modulo, 33, "[null, 1, 3, null, 2]", "[null, 0, 0, null,
1]");
+ // Array % Scalar
+ this->AssertBinop(Modulo, "[null, 10, 30, null, 2]", 3, "[null, 1, 0,
null, 2]");
+ }
+}
+
+TYPED_TEST(TestBinaryArithmeticSigned, Modulo) {
+ // Floored semantics: sign follows divisor
+ this->AssertBinop(Modulo, "[7]", "[3]", "[1]");
+ this->AssertBinop(Modulo, "[-7]", "[3]", "[2]");
+ this->AssertBinop(Modulo, "[7]", "[-3]", "[-2]");
+ this->AssertBinop(Modulo, "[-7]", "[-3]", "[-1]");
+ // Edge case: -1 mod positive
Review Comment:
another nitpick: modulo (or "negative dividend and positive divisor")
##########
docs/source/cpp/compute.rst:
##########
@@ -566,6 +575,18 @@ Mixed time resolution temporal inputs will be cast to
finest input resolution.
intermediate stages of the computation. If either argument is infinite, the
result is ``+Inf`` even if the other argument is NaN.
+* \(4) Computes the floored modulo, where the result has the same sign as the
+ divisor. This is equivalent to Python's ``%`` operator. Integer and decimal
+ division by zero returns an error, while floating-point division by zero
+ returns NaN. Decimal arguments are promoted to a common scale ``s``; the
+ result then has ``scale = s`` and ``precision = max(p1, p2)``.
Review Comment:
Perhaps a change like:
```rst
Division by zero returns an error for integer and decimal inputs. For
floating-point inputs it returns ``NaN`` in ``modulo`` and an error in
``modulo_checked``.
```
--
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]
