From 9901d1f8858056f707b8ad7a187929205fb9ad71 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sun, 13 Oct 2024 13:13:57 -0400 Subject: [PATCH] linting and doc fixes --- .../arrow/compute/kernels/aggregate_basic.cc | 11 ++-- .../arrow/compute/kernels/aggregate_test.cc | 50 ++++++++++++------- .../arrow/compute/kernels/codegen_internal.cc | 13 +++-- .../arrow/compute/kernels/codegen_internal.h | 5 +- cpp/src/arrow/compute/kernels/test_util.h | 20 ++++---- 5 files changed, 60 insertions(+), 39 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic.cc b/cpp/src/arrow/compute/kernels/aggregate_basic.cc index fe48b31b6b8a2..1477f42d94e6c 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic.cc @@ -1053,10 +1053,10 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) { func = std::make_shared("sum", Arity::Unary(), sum_doc, &default_scalar_aggregate_options); AddArrayScalarAggKernels(SumInit, {boolean()}, uint64(), func.get()); - AddAggKernel(KernelSignature::Make({Type::DECIMAL32}, MaxPrecisionDecimalType), - SumInit, func.get(), SimdLevel::NONE); - AddAggKernel(KernelSignature::Make({Type::DECIMAL64}, MaxPrecisionDecimalType), - SumInit, func.get(), SimdLevel::NONE); + AddAggKernel(KernelSignature::Make({Type::DECIMAL32}, MaxPrecisionDecimalType), SumInit, + func.get(), SimdLevel::NONE); + AddAggKernel(KernelSignature::Make({Type::DECIMAL64}, MaxPrecisionDecimalType), SumInit, + func.get(), SimdLevel::NONE); AddAggKernel(KernelSignature::Make({Type::DECIMAL128}, MaxPrecisionDecimalType), SumInit, func.get(), SimdLevel::NONE); AddAggKernel(KernelSignature::Make({Type::DECIMAL256}, MaxPrecisionDecimalType), @@ -1204,7 +1204,8 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) { AddBasicAggKernels(IndexInit::Init, PrimitiveTypes(), int64(), func.get()); AddBasicAggKernels(IndexInit::Init, TemporalTypes(), int64(), func.get()); AddBasicAggKernels(IndexInit::Init, - {fixed_size_binary(1), decimal32(1, 0), decimal64(1, 0), decimal128(1, 0), decimal256(1, 0), null()}, + {fixed_size_binary(1), decimal32(1, 0), decimal64(1, 0), + decimal128(1, 0), decimal256(1, 0), null()}, int64(), func.get()); DCHECK_OK(registry->AddFunction(std::move(func))); } diff --git a/cpp/src/arrow/compute/kernels/aggregate_test.cc b/cpp/src/arrow/compute/kernels/aggregate_test.cc index f134d8edb8c53..e3b2f9c17f0bf 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_test.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_test.cc @@ -494,8 +494,10 @@ TEST_F(TestSumKernelRoundOff, Basics) { } TEST(TestDecimalSumKernel, SimpleSum) { - std::vector> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; - std::vector> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; + std::vector> init_types = { + decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; + std::vector> out_types = { + decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; for (size_t i = 0; i < init_types.size(); ++i) { auto& ty = init_types[i]; @@ -541,12 +543,14 @@ TEST(TestDecimalSumKernel, SimpleSum) { } TEST(TestDecimalSumKernel, ScalarAggregateOptions) { - std::vector> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; - std::vector> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; + std::vector> init_types = { + decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; + std::vector> out_types = { + decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; for (size_t i = 0; i < init_types.size(); ++i) { auto& ty = init_types[i]; - auto& out_ty = out_types[i ]; + auto& out_ty = out_types[i]; Datum null = ScalarFromJSON(out_ty, R"(null)"); Datum zero = ScalarFromJSON(out_ty, R"("0.00")"); @@ -723,12 +727,14 @@ TYPED_TEST(TestNumericProductKernel, ScalarAggregateOptions) { } TEST(TestDecimalProductKernel, SimpleProduct) { - std::vector> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; - std::vector> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; + std::vector> init_types = { + decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; + std::vector> out_types = { + decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; for (size_t i = 0; i < init_types.size(); ++i) { auto& ty = init_types[i]; - auto& out_ty = out_types[i ]; + auto& out_ty = out_types[i]; Datum null = ScalarFromJSON(out_ty, R"(null)"); @@ -754,7 +760,8 @@ TEST(TestDecimalProductKernel, SimpleProduct) { EXPECT_THAT(Product(ArrayFromJSON(ty, R"([null])"), options), ResultWith(ScalarFromJSON(out_ty, R"("1.00")"))); chunks = ChunkedArrayFromJSON(ty, {}); - EXPECT_THAT(Product(chunks, options), ResultWith(ScalarFromJSON(out_ty, R"("1.00")"))); + EXPECT_THAT(Product(chunks, options), + ResultWith(ScalarFromJSON(out_ty, R"("1.00")"))); EXPECT_THAT(Product(ArrayFromJSON( ty, R"(["1.00", null, "-3.00", null, "3.00", null, "7.00"])"), @@ -768,12 +775,14 @@ TEST(TestDecimalProductKernel, SimpleProduct) { } TEST(TestDecimalProductKernel, ScalarAggregateOptions) { - std::vector> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; - std::vector> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; + std::vector> init_types = { + decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; + std::vector> out_types = { + decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; for (size_t i = 0; i < init_types.size(); ++i) { auto& ty = init_types[i]; - auto& out_ty = out_types[i ]; + auto& out_ty = out_types[i]; Datum null = ScalarFromJSON(out_ty, R"(null)"); Datum one = ScalarFromJSON(out_ty, R"("1.00")"); @@ -1360,8 +1369,10 @@ TYPED_TEST(TestRandomNumericMeanKernel, RandomArrayMeanOverflow) { TEST(TestDecimalMeanKernel, SimpleMean) { ScalarAggregateOptions options(/*skip_nulls=*/true, /*min_count=*/0); - std::vector> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; - std::vector> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; + std::vector> init_types = { + decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; + std::vector> out_types = { + decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; for (size_t i = 0; i < init_types.size(); ++i) { auto& ty = init_types[i]; @@ -1421,8 +1432,9 @@ TEST(TestDecimalMeanKernel, SimpleMean) { } // TODO: Currently, casts are not implemented for decimal32/64 so we ignore that for now - // init_types = {decimal32(3, -2), decimal64(3, -2), decimal128(3, -2), decimal256(3, -2)}; - // out_types = {decimal32(9, -2), decimal64(18, -2), decimal128(38, -2), decimal256(76, -2)}; + // init_types = {decimal32(3, -2), decimal64(3, -2), decimal128(3, -2), decimal256(3, + // -2)}; out_types = {decimal32(9, -2), decimal64(18, -2), decimal128(38, -2), + // decimal256(76, -2)}; init_types = {decimal128(3, -2), decimal256(3, -2)}; out_types = {decimal128(38, -2), decimal256(76, -2)}; @@ -1463,8 +1475,10 @@ TEST(TestDecimalMeanKernel, SimpleMean) { } TEST(TestDecimalMeanKernel, ScalarAggregateOptions) { - std::vector> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; - std::vector> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; + std::vector> init_types = { + decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)}; + std::vector> out_types = { + decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)}; for (size_t i = 0; i < init_types.size(); ++i) { auto& ty = init_types[i]; diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.cc b/cpp/src/arrow/compute/kernels/codegen_internal.cc index cbb875a1038fe..5b31cdc03280b 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.cc +++ b/cpp/src/arrow/compute/kernels/codegen_internal.cc @@ -78,7 +78,8 @@ Result ListValuesType(KernelContext* ctx, Result MaxPrecisionDecimalType(KernelContext*, const std::vector& args) { - ARROW_ASSIGN_OR_RAISE(auto out_type_, WidenDecimalToMaxPrecision(args[0].GetSharedPtr())); + ARROW_ASSIGN_OR_RAISE(auto out_type_, + WidenDecimalToMaxPrecision(args[0].GetSharedPtr())); return out_type_; } @@ -533,9 +534,11 @@ Status CastDecimalArgs(TypeHolder* begin, size_t count) { return Status::OK(); } -Result> WidenDecimalToMaxPrecision(std::shared_ptr type) { +Result> WidenDecimalToMaxPrecision( + std::shared_ptr type) { if (!is_decimal(type->id())) { - return Status::TypeError("Non-DecimalType passed to WidenDecimalToMaxPrecision: " + type->ToString()); + return Status::TypeError("Non-DecimalType passed to WidenDecimalToMaxPrecision: " + + type->ToString()); } auto cast_type = checked_pointer_cast(type); switch (type->id()) { @@ -548,7 +551,9 @@ Result> WidenDecimalToMaxPrecision(std::shared_ptrscale()); default: - return Status::TypeError("An unknown DecimalType was passed to WidenDecimalToMaxPrecision: " + type->ToString()); + return Status::TypeError( + "An unknown DecimalType was passed to WidenDecimalToMaxPrecision: " + + type->ToString()); }; } diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.h b/cpp/src/arrow/compute/kernels/codegen_internal.h index 952ce0bbd4e9a..f62b69e2f4629 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.h +++ b/cpp/src/arrow/compute/kernels/codegen_internal.h @@ -1446,9 +1446,10 @@ Status CastDecimalArgs(TypeHolder* begin, size_t count); /// Given a DataType, if it is a DecimalType, return a DecimalType with the same scale /// and the maximum precision for that DecimalType. /// -/// If it is _not_ a DecimalType, return type +/// If it is _not_ a DecimalType, return a Status::TypeError ARROW_EXPORT -Result> WidenDecimalToMaxPrecision(std::shared_ptr type); +Result> WidenDecimalToMaxPrecision( + std::shared_ptr type); ARROW_EXPORT bool HasDecimal(const std::vector& types); diff --git a/cpp/src/arrow/compute/kernels/test_util.h b/cpp/src/arrow/compute/kernels/test_util.h index 3bcf5d38f30b9..11e44283ac567 100644 --- a/cpp/src/arrow/compute/kernels/test_util.h +++ b/cpp/src/arrow/compute/kernels/test_util.h @@ -78,11 +78,11 @@ inline std::shared_ptr DecimalArrayFromJSON(const std::shared_ptr(*type); if (ty.scale() >= 0) return ArrayFromJSON(type, json); auto p = ty.precision() - ty.scale(); - std::shared_ptr adjusted_ty = - ty.id() == Type::DECIMAL32 ? decimal32(p, 0) : - ty.id() == Type::DECIMAL64 ? decimal64(p, 0) : - ty.id() == Type::DECIMAL128 ? decimal128(p, 0) : - decimal256(p, 0); + std::shared_ptr adjusted_ty = ty.id() == Type::DECIMAL32 ? decimal32(p, 0) + : ty.id() == Type::DECIMAL64 ? decimal64(p, 0) + : ty.id() == Type::DECIMAL128 + ? decimal128(p, 0) + : decimal256(p, 0); return Cast(ArrayFromJSON(adjusted_ty, json), type).ValueOrDie().make_array(); } @@ -91,11 +91,11 @@ inline std::shared_ptr DecimalScalarFromJSON( const auto& ty = checked_cast(*type); if (ty.scale() >= 0) return ScalarFromJSON(type, json); auto p = ty.precision() - ty.scale(); - std::shared_ptr adjusted_ty = - ty.id() == Type::DECIMAL32 ? decimal32(p, 0) : - ty.id() == Type::DECIMAL64 ? decimal64(p, 0) : - ty.id() == Type::DECIMAL128 ? decimal128(p, 0) : - decimal256(p, 0); + std::shared_ptr adjusted_ty = ty.id() == Type::DECIMAL32 ? decimal32(p, 0) + : ty.id() == Type::DECIMAL64 ? decimal64(p, 0) + : ty.id() == Type::DECIMAL128 + ? decimal128(p, 0) + : decimal256(p, 0); return Cast(ScalarFromJSON(adjusted_ty, json), type).ValueOrDie().scalar(); }