cyb70289 commented on a change in pull request #11313:
URL: https://github.com/apache/arrow/pull/11313#discussion_r724103120
##########
File path: cpp/src/arrow/compute/kernels/aggregate_tdigest.cc
##########
@@ -121,7 +134,7 @@ struct TDigestInitState {
TDigestInitState(KernelContext* ctx, const DataType& in_type,
const TDigestOptions& options)
- : ctx(ctx), in_type(in_type), options(options) {}
+ : ctx(ctx), in_type(std::move(in_type)), options(options) {}
Review comment:
Change parameter `in_type` (and member variable `in_type`) from
reference to `DataType in_type`?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_var_std.cc
##########
@@ -52,14 +61,13 @@ struct VarStdState {
return;
}
- using SumType =
- typename std::conditional<is_floating_type<T>::value, double,
int128_t>::type;
- SumType sum = SumArray<CType, SumType, SimdLevel::NONE>(*array.data());
+ using SumType = typename internal::GetSumType<T>::SumType;
+ SumType sum = internal::SumArray<CType, SumType,
SimdLevel::NONE>(*array.data());
- const double mean = static_cast<double>(sum) / count;
- const double m2 =
- SumArray<CType, double, SimdLevel::NONE>(*array.data(), [mean](CType
value) {
- const double v = static_cast<double>(value);
+ const double mean = ToDouble(sum) / count;
+ const double m2 = internal::SumArray<CType, double, SimdLevel::NONE>(
+ *array.data(), [&, mean](CType value) {
Review comment:
Capture `[this, mean]`?
Or `[&]` only?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_tdigest.cc
##########
@@ -34,13 +34,25 @@ template <typename ArrowType>
struct TDigestImpl : public ScalarAggregator {
using ThisType = TDigestImpl<ArrowType>;
using ArrayType = typename TypeTraits<ArrowType>::ArrayType;
- using CType = typename ArrowType::c_type;
+ using CType = typename TypeTraits<ArrowType>::CType;
- explicit TDigestImpl(const TDigestOptions& options)
+ explicit TDigestImpl(const TDigestOptions& options, const DataType& in_type)
Review comment:
Nit: `explicit` can be removed as we have two parameters now.
--
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]
