pitrou commented on a change in pull request #11313:
URL: https://github.com/apache/arrow/pull/11313#discussion_r741960401
##########
File path: docs/source/cpp/compute.rst
##########
@@ -1113,26 +1125,28 @@ Containment tests
Categorizations
~~~~~~~~~~~~~~~
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| Function name | Arity | Input types | Output type |
Options class | Notes |
-+===================+============+=====================+=====================+========================+=========+
-| is_finite | Unary | Float, Double | Boolean |
| \(1) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_inf | Unary | Float, Double | Boolean |
| \(2) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_nan | Unary | Float, Double | Boolean |
| \(3) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_null | Unary | Any | Boolean |
:struct:`NullOptions` | \(4) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_valid | Unary | Any | Boolean |
| \(5) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
++-------------------+------------+-------------------------+---------------------+------------------------+---------+
+| Function name | Arity | Input types | Output type
| Options class | Notes |
++===================+============+=========================+=====================+========================+=========+
+| is_finite | Unary | Float32/Float64/Decimal | Boolean
| | \(1) |
++-------------------+------------+-------------------------+---------------------+------------------------+---------+
+| is_inf | Unary | Float32/Float64/Decimal | Boolean
| | \(2) |
++-------------------+------------+-------------------------+---------------------+------------------------+---------+
+| is_nan | Unary | Float32/Float64/Decimal | Boolean
| | \(3) |
Review comment:
It also accepts ints and nans apparently, so perhaps "Numeric, Null"?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_mode.cc
##########
@@ -378,6 +395,21 @@ VectorKernel NewModeKernel(const
std::shared_ptr<DataType>& in_type) {
return kernel;
}
+Result<ValueDescr> ModeType(KernelContext*, const std::vector<ValueDescr>&
descrs) {
+ return ValueDescr::Array(
+ struct_({field(kModeFieldName, descrs[0].type), field(kCountFieldName,
int64())}));
+}
+
+VectorKernel NewModeKernel(Type::type type_id, ArrayKernelExec exec) {
Review comment:
Nit: perhaps define a single `NewModeKernel(InputType, OutputType,
ArrayKernelExec)`?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_test.cc
##########
@@ -3541,6 +3649,24 @@ TEST(TestQuantileKernel, AllNullsOrNaNs) {
}
}
+TEST(TestQuantileKernel, Decimal) {
+ auto check = [](const std::shared_ptr<Array>& input, QuantileOptions options,
+ const std::shared_ptr<Array>& expected) {
+ ASSERT_OK_AND_ASSIGN(Datum out, Quantile(input, options));
+ auto out_array = out.make_array();
+ ValidateOutput(*out_array);
+ AssertArraysEqual(*expected, *out_array, /*verbose=*/true);
+ };
+ for (const auto& ty : {decimal128(3, 2), decimal256(3, 2)}) {
+ check(ArrayFromJSON(ty, R"(["1.00", "5.00", null])"),
+ QuantileOptions(0.5, QuantileOptions::LINEAR),
+ ArrayFromJSON(float64(), R"([3.00])"));
Review comment:
Hmm, should LINEAR return the input decimal type instead of float64?
##########
File path: docs/source/cpp/compute.rst
##########
@@ -1113,26 +1125,28 @@ Containment tests
Categorizations
~~~~~~~~~~~~~~~
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| Function name | Arity | Input types | Output type |
Options class | Notes |
-+===================+============+=====================+=====================+========================+=========+
-| is_finite | Unary | Float, Double | Boolean |
| \(1) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_inf | Unary | Float, Double | Boolean |
| \(2) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_nan | Unary | Float, Double | Boolean |
| \(3) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_null | Unary | Any | Boolean |
:struct:`NullOptions` | \(4) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
-| is_valid | Unary | Any | Boolean |
| \(5) |
-+-------------------+------------+---------------------+---------------------+------------------------+---------+
++-------------------+------------+-------------------------+---------------------+------------------------+---------+
+| Function name | Arity | Input types | Output type
| Options class | Notes |
++===================+============+=========================+=====================+========================+=========+
+| is_finite | Unary | Float32/Float64/Decimal | Boolean
| | \(1) |
++-------------------+------------+-------------------------+---------------------+------------------------+---------+
+| is_inf | Unary | Float32/Float64/Decimal | Boolean
| | \(2) |
++-------------------+------------+-------------------------+---------------------+------------------------+---------+
+| is_nan | Unary | Float32/Float64/Decimal | Boolean
| | \(3) |
Review comment:
It also accepts ints and nans apparently, so perhaps "Numeric, Null"?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_mode.cc
##########
@@ -378,6 +395,21 @@ VectorKernel NewModeKernel(const
std::shared_ptr<DataType>& in_type) {
return kernel;
}
+Result<ValueDescr> ModeType(KernelContext*, const std::vector<ValueDescr>&
descrs) {
+ return ValueDescr::Array(
+ struct_({field(kModeFieldName, descrs[0].type), field(kCountFieldName,
int64())}));
+}
+
+VectorKernel NewModeKernel(Type::type type_id, ArrayKernelExec exec) {
Review comment:
Nit: perhaps define a single `NewModeKernel(InputType, OutputType,
ArrayKernelExec)`?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_test.cc
##########
@@ -3541,6 +3649,24 @@ TEST(TestQuantileKernel, AllNullsOrNaNs) {
}
}
+TEST(TestQuantileKernel, Decimal) {
+ auto check = [](const std::shared_ptr<Array>& input, QuantileOptions options,
+ const std::shared_ptr<Array>& expected) {
+ ASSERT_OK_AND_ASSIGN(Datum out, Quantile(input, options));
+ auto out_array = out.make_array();
+ ValidateOutput(*out_array);
+ AssertArraysEqual(*expected, *out_array, /*verbose=*/true);
+ };
+ for (const auto& ty : {decimal128(3, 2), decimal256(3, 2)}) {
+ check(ArrayFromJSON(ty, R"(["1.00", "5.00", null])"),
+ QuantileOptions(0.5, QuantileOptions::LINEAR),
+ ArrayFromJSON(float64(), R"([3.00])"));
Review comment:
Hmm, should LINEAR return the input decimal type instead of float64?
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
