bkietz commented on a change in pull request #10113:
URL: https://github.com/apache/arrow/pull/10113#discussion_r617671811
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -817,5 +933,126 @@ TEST(TestBinaryArithmetic,
AddWithImplicitCastsUint64EdgeCase) {
ArrayFromJSON(uint64(),
"[18446744073709551615]")}));
}
+TEST(TestUnaryArithmetic, DispatchBest) {
+ for (std::string name : {"negate"}) {
+ for (const auto& ty : {int8(), int16(), int32(), int64(), uint8(),
uint16(), uint32(),
+ uint64(), float32(), float64()}) {
+ CheckDispatchBest(name, {ty}, {ty});
+ CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
+ }
+ }
+
+ 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});
+ }
+ }
+}
+
+TYPED_TEST(TestUnaryArithmeticSigned, Negate) {
+ using CType = typename TestFixture::CType;
+
+ auto min = std::numeric_limits<CType>::min();
+ auto max = std::numeric_limits<CType>::max();
+
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertUnaryOp(Negate, "[]", "[]");
+ // Array with nulls
+ this->AssertUnaryOp(Negate, "[null]", "[null]");
+ this->AssertUnaryOp(Negate, this->MakeNullScalar(), "[null]");
Review comment:
```suggestion
this->AssertUnaryOp(Negate, this->MakeNullScalar(),
this->MakeNullScalar());
```
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -54,6 +54,117 @@ std::shared_ptr<Array> TweakValidityBit(const
std::shared_ptr<Array>& array,
return MakeArray(data);
}
+template <typename T>
+class TestUnaryArithmetic : public TestBase {
+ protected:
+ using ArrowType = T;
+ using CType = typename ArrowType::c_type;
+
+ static std::shared_ptr<DataType> type_singleton() {
+ return TypeTraits<ArrowType>::type_singleton();
+ }
+
+ using UnaryFunction =
+ std::function<Result<Datum>(const Datum&, ArithmeticOptions,
ExecContext*)>;
+
+ void SetUp() override { options_.check_overflow = false; }
+
+ std::shared_ptr<Scalar> MakeNullScalar() {
+ return arrow::MakeNullScalar(type_singleton());
+ }
+
+ std::shared_ptr<Scalar> MakeScalar(CType value) {
+ return *arrow::MakeScalar(type_singleton(), value);
+ }
+
+ // (Scalar)
+ void AssertUnaryOp(UnaryFunction func, CType argument, CType expected) {
+ auto arg = MakeScalar(argument);
+ auto exp = MakeScalar(expected);
+ ASSERT_OK_AND_ASSIGN(auto actual, func(arg, options_, nullptr));
+ AssertScalarsApproxEqual(*exp, *actual.scalar(), /*verbose=*/true);
+ }
+
+ // (Scalar)
+ void AssertUnaryOp(UnaryFunction func, const std::shared_ptr<Scalar>& arg,
+ const std::string& expected) {
+ auto exp = ArrayFromJSON(type_singleton(), expected);
+ ASSERT_OK_AND_ASSIGN(auto actual, func(arg, options_, nullptr));
+ AssertScalarsApproxEqual(*(*exp->GetScalar(0)), *actual.scalar(),
/*verbose=*/true);
Review comment:
Instead of going through JSON, please just provide scalars
```suggestion
const std::shared_ptr<Scalar>& expected) {
ASSERT_OK_AND_ASSIGN(auto actual, func(arg, options_, nullptr));
AssertScalarsApproxEqual(*expected, *actual.scalar(), /*verbose=*/true);
```
##########
File path: docs/source/cpp/compute.rst
##########
@@ -268,14 +268,18 @@ an ``Invalid`` :class:`Status` when overflow is detected.
+--------------------------+------------+--------------------+---------------------+
| divide_checked | Binary | Numeric | Numeric
|
+--------------------------+------------+--------------------+---------------------+
-| power | Binary | Numeric | Numeric
|
-+--------------------------+------------+--------------------+---------------------+
-| power_checked | Binary | Numeric | Numeric
|
-+--------------------------+------------+--------------------+---------------------+
| multiply | Binary | Numeric | Numeric
|
+--------------------------+------------+--------------------+---------------------+
| multiply_checked | Binary | Numeric | Numeric
|
+--------------------------+------------+--------------------+---------------------+
+| negate | Unary | Numeric | Numeric
|
++--------------------------+------------+--------------------+---------------------+
+| negate_checked | Unary | Numeric | Numeric
|
Review comment:
```suggestion
| negate_checked | Unary | Signed Numeric | Signed
Numeric |
```
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -369,12 +420,37 @@ std::shared_ptr<ScalarFunction>
MakeArithmeticFunctionNotNull(std::string name,
const
FunctionDoc* doc) {
auto func = std::make_shared<ArithmeticFunction>(name, Arity::Binary(), doc);
for (const auto& ty : NumericTypes()) {
- auto exec = NumericEqualTypesBinary<ScalarBinaryNotNullEqualTypes, Op>(ty);
+ auto exec = ArithmeticExecFromOp<ScalarBinaryNotNullEqualTypes, Op>(ty);
DCHECK_OK(func->AddKernel({ty, ty}, ty, exec));
}
return func;
}
+template <typename Op>
+std::shared_ptr<ScalarFunction> MakeUnaryArithmeticFunction(std::string name,
+ const FunctionDoc*
doc) {
+ auto func = std::make_shared<ArithmeticFunction>(name, Arity::Unary(), doc);
+ for (const auto& ty : NumericTypes()) {
+ auto exec = ArithmeticExecFromOp<ScalarUnary, Op>(ty);
+ DCHECK_OK(func->AddKernel({ty}, ty, exec));
+ }
+ return func;
+}
+
+// Like MakeUnaryArithmeticFunction, but for signed arithmetic ops that need
to run
+// only on non-null output.
+template <typename Op>
+std::shared_ptr<ScalarFunction>
MakeUnaryCheckedSignedArithmeticFunctionNotNull(
+ std::string name, const FunctionDoc* doc) {
+ auto func = std::make_shared<ArithmeticFunction>(name, Arity::Unary(), doc);
+ for (const auto& ty :
arrow::internal::FlattenVectors<std::shared_ptr<arrow::DataType>>(
+ {SignedIntTypes(), FloatingPointTypes()})) {
Review comment:
Slightly simpler:
```suggestion
for (const auto& ty : NumericTypes()) {
if (is_unsigned_integer(ty->id())) {
// don't support negate_checked for unsigned integers
continue;
}
```
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -817,5 +933,126 @@ TEST(TestBinaryArithmetic,
AddWithImplicitCastsUint64EdgeCase) {
ArrayFromJSON(uint64(),
"[18446744073709551615]")}));
}
+TEST(TestUnaryArithmetic, DispatchBest) {
+ for (std::string name : {"negate"}) {
+ for (const auto& ty : {int8(), int16(), int32(), int64(), uint8(),
uint16(), uint32(),
+ uint64(), float32(), float64()}) {
+ CheckDispatchBest(name, {ty}, {ty});
+ CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty});
+ }
+ }
+
+ 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});
+ }
+ }
+}
+
+TYPED_TEST(TestUnaryArithmeticSigned, Negate) {
+ using CType = typename TestFixture::CType;
+
+ auto min = std::numeric_limits<CType>::min();
+ auto max = std::numeric_limits<CType>::max();
+
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ // Empty arrays
+ this->AssertUnaryOp(Negate, "[]", "[]");
+ // Array with nulls
+ this->AssertUnaryOp(Negate, "[null]", "[null]");
+ this->AssertUnaryOp(Negate, this->MakeNullScalar(), "[null]");
+ this->AssertUnaryOp(Negate, "[1, null, -10]", "[-1, null, 10]");
+ // Arrays with zeros
+ this->AssertUnaryOp(Negate, "[0, 0, -0]", "[0, -0, 0]");
+ this->AssertUnaryOp(Negate, 0, -0);
+ this->AssertUnaryOp(Negate, -0, 0);
+ this->AssertUnaryOp(Negate, 0, 0);
+ // Ordinary arrays (positive inputs)
+ this->AssertUnaryOp(Negate, "[1, 10, 127]", "[-1, -10, -127]");
+ this->AssertUnaryOp(Negate, 1, -1);
+ this->AssertUnaryOp(Negate, this->MakeScalar(1), "[-1]");
Review comment:
```suggestion
this->AssertUnaryOp(Negate, this->MakeScalar(1), this->MakeScalar(-1));
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
