Copilot commented on code in PR #48914:
URL: https://github.com/apache/arrow/pull/48914#discussion_r3907965441
##########
cpp/src/arrow/compute/api_scalar.h:
##########
@@ -671,6 +671,40 @@ Result<Datum> Divide(const Datum& left, const Datum& right,
ArithmeticOptions options = ArithmeticOptions(),
ExecContext* ctx = NULLPTR);
+/// \brief Compute the remainder (truncated division) of two values.
+/// Array values must be the same length. If either argument is null the result
+/// will be null. For integer types, if there is a zero divisor, an error will
be
+/// raised.
+///
+/// The result has the same sign as the dividend (C/C++ semantics).
+///
+/// \param[in] left the dividend
+/// \param[in] right the divisor
+/// \param[in] options arithmetic options (enable/disable overflow checking),
optional
+/// \param[in] ctx the function execution context, optional
+/// \return the elementwise remainder
+ARROW_EXPORT
+Result<Datum> Remainder(const Datum& left, const Datum& right,
+ ArithmeticOptions options = ArithmeticOptions(),
+ ExecContext* ctx = NULLPTR);
+
+/// \brief Compute the modulo (floored division) of two values.
+/// Array values must be the same length. If either argument is null the result
+/// will be null. For integer types, if there is a zero divisor, an error will
be
+/// raised.
Review Comment:
This comment says divide-by-zero errors apply only to integer types, but
`modulo` also raises `Invalid` on zero divisor for decimal inputs. Update the
API doc to include decimals (and avoid implying floats/decimals behave like
integers here).
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:
##########
@@ -1173,6 +1189,40 @@ const FunctionDoc div_checked_doc{
"integer overflow is encountered."),
{"dividend", "divisor"}};
+const FunctionDoc remainder_doc{
+ "Compute the remainder after integer division (truncated)",
+ ("Returns the remainder after dividing the dividend by the divisor.\n"
+ "The result has the same sign as the dividend (truncated division).\n"
+ "This is equivalent to the C/C++ '%' operator.\n"
+ "Integer division by zero returns an error."),
+ {"dividend", "divisor"}};
+
+const FunctionDoc remainder_checked_doc{
+ "Compute the remainder after integer division (truncated)",
+ ("Returns the remainder after dividing the dividend by the divisor.\n"
+ "The result has the same sign as the dividend (truncated division).\n"
+ "This is equivalent to the C/C++ '%' operator.\n"
+ "An error is returned when trying to divide by zero, or when\n"
+ "integer overflow is encountered."),
Review Comment:
These docs describe `remainder` as operating on “integer division” and
suggest divide-by-zero errors only for integer inputs, but the kernel also
supports floating-point and decimal inputs (floats yield NaN on div-by-zero in
the unchecked variant; decimals error). Please adjust wording to match actual
behavior, and apply the same wording to `remainder_checked`.
This issue also appears on line 1210 of the same file.
##########
docs/source/cpp/compute.rst:
##########
@@ -514,6 +515,10 @@ Mixed time resolution temporal inputs will be cast to
finest input resolution.
+------------------+--------+-------------------------+-------------------------------+-------+
| hypot | Binary | Numeric | Float32/Float64
| \(3) |
+------------------+--------+-------------------------+-------------------------------+-------+
+| modulo | Binary | Numeric | Numeric
| \(4) |
++------------------+--------+-------------------------+-------------------------------+-------+
+| modulo_checked | Binary | Numeric | Numeric
| \(4) |
++------------------+--------+-------------------------+-------------------------------+-------+
Review Comment:
The PR description says it adds kernels `mod` and `mod_checked`, but this
documentation (and the C++ registry/API in this PR) uses `modulo` /
`modulo_checked`. Please reconcile this (either add `mod` aliases or update the
PR description / any external references to the actual names).
--
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]
