zanmato1984 commented on code in PR #55291:
URL: https://github.com/apache/arrow/pull/55291#discussion_r4428965507
##########
cpp/src/arrow/compute/kernels/aggregate_quantile.cc:
##########
@@ -531,14 +531,14 @@ void AddQuantileKernels(VectorFunction* func) {
for (const auto& ty : NumericTypes()) {
base.signature = KernelSignature::Make({InputType(ty)},
OutputType(ResolveOutput));
// output type is determined at runtime, set template argument to nulltype
Review Comment:
Nit: the `nulltype` placeholder is no longer passed here. Could we simplify
this comment to “The output type is determined at runtime by `ResolveOutput`”?
##########
cpp/src/arrow/compute/kernels/codegen_internal.h:
##########
@@ -1016,8 +1018,8 @@ using ScalarBinaryNotNullStatefulEqualTypes =
// subclass corresponding to the type passed as argument (not template type) to
// the function.
//
-// For example, GenerateNumeric<FUNCTOR, Type0>(int32()) will select a kernel
-// instantiated like FUNCTOR<Type0, Int32Type>. Any additional variadic
+// For example, GenerateNumeric<FUNCTOR>(int32()) will select a kernel
+// instantiated like FUNCTOR<Int32Type>. Any additional variadic
Review Comment:
Nit: could we also update the overview above? It still says a fixed `Type0`
argument is required and precedes the selected type, whereas the default is now
`FUNCTOR<Type, Args...>`. It would also help to mention `WithType0<T>` and
`WithFirstTypeTwice` for the alternative layouts.
--
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]
