Skip to content

Commit ccfb5fd

Browse files
lidavidmlidavidm
committed
ARROW-13096: [C++] Improve error messages/tests
1 parent ff007b7 commit ccfb5fd

3 files changed

Lines changed: 60 additions & 49 deletions

File tree

‎cpp/src/arrow/compute/api_scalar.h‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,9 @@ class ARROW_EXPORT ProjectOptions : public FunctionOptions {
246246

247247
/// @}
248248

249-
/// \brief Get the absolute value of a value. Array values can be of arbitrary
250-
/// length. If argument is null the result will be null.
249+
/// \brief Get the absolute value of a value.
250+
///
251+
/// If argument is null the result will be null.
251252
///
252253
/// \param[in] arg the value transformed
253254
/// \param[in] options arithmetic options (overflow handling), optional
@@ -311,8 +312,9 @@ Result<Datum> Divide(const Datum& left, const Datum& right,
311312
ArithmeticOptions options = ArithmeticOptions(),
312313
ExecContext* ctx = NULLPTR);
313314

314-
/// \brief Negate a value. Array values can be of arbitrary length. If argument
315-
/// is null the result will be null.
315+
/// \brief Negate values.
316+
///
317+
/// If argument is null the result will be null.
316318
///
317319
/// \param[in] arg the value negated
318320
/// \param[in] options arithmetic options (overflow handling), optional
@@ -424,45 +426,48 @@ Result<Datum> Atan(const Datum& arg, ExecContext* ctx = NULLPTR);
424426
ARROW_EXPORT
425427
Result<Datum> Atan2(const Datum& y, const Datum& x, ExecContext* ctx = NULLPTR);
426428

427-
/// \brief Get the natural log of a value. Array values can be of arbitrary
428-
/// length. If argument is null the result will be null.
429+
/// \brief Get the natural log of a value.
429430
///
430-
/// \param[in] arg the value transformed
431+
/// If argument is null the result will be null.
432+
///
433+
/// \param[in] arg The values to compute the logarithm for.
431434
/// \param[in] options arithmetic options (overflow handling), optional
432435
/// \param[in] ctx the function execution context, optional
433436
/// \return the elementwise natural log
434437
ARROW_EXPORT
435438
Result<Datum> Ln(const Datum& arg, ArithmeticOptions options = ArithmeticOptions(),
436439
ExecContext* ctx = NULLPTR);
437440

438-
/// \brief Get the log base 10 of a value. Array values can be of arbitrary
439-
/// length. If argument is null the result will be null.
441+
/// \brief Get the log base 10 of a value.
440442
///
441-
/// \param[in] arg the value transformed
443+
/// If argument is null the result will be null.
444+
///
445+
/// \param[in] arg The values to compute the logarithm for.
442446
/// \param[in] options arithmetic options (overflow handling), optional
443447
/// \param[in] ctx the function execution context, optional
444448
/// \return the elementwise log base 10
445449
ARROW_EXPORT
446450
Result<Datum> Log10(const Datum& arg, ArithmeticOptions options = ArithmeticOptions(),
447451
ExecContext* ctx = NULLPTR);
448452

449-
/// \brief Get the log base 2 of a value. Array values can be of arbitrary
450-
/// length. If argument is null the result will be null.
453+
/// \brief Get the log base 2 of a value.
451454
///
452-
/// \param[in] arg the value transformed
455+
/// If argument is null the result will be null.
456+
///
457+
/// \param[in] arg The values to compute the logarithm for.
453458
/// \param[in] options arithmetic options (overflow handling), optional
454459
/// \param[in] ctx the function execution context, optional
455460
/// \return the elementwise log base 2
456461
ARROW_EXPORT
457462
Result<Datum> Log2(const Datum& arg, ArithmeticOptions options = ArithmeticOptions(),
458463
ExecContext* ctx = NULLPTR);
459464

460-
/// \brief Get the natural log of (1 + value). Array values can be of arbitrary
461-
/// length. If argument is null the result will be null.
465+
/// \brief Get the natural log of (1 + value).
462466
///
467+
/// If argument is null the result will be null.
463468
/// This function may be more accurate than Log(1 + value) for values close to zero.
464469
///
465-
/// \param[in] arg the value transformed
470+
/// \param[in] arg The values to compute the logarithm for.
466471
/// \param[in] options arithmetic options (overflow handling), optional
467472
/// \param[in] ctx the function execution context, optional
468473
/// \return the elementwise natural log

‎cpp/src/arrow/compute/kernels/scalar_arithmetic.cc‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -704,10 +704,10 @@ struct LogNaturalChecked {
704704
static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg, Status* st) {
705705
static_assert(std::is_same<T, Arg>::value, "");
706706
if (arg == 0.0) {
707-
*st = Status::Invalid("divide by zero");
707+
*st = Status::Invalid("logarithm of zero");
708708
return arg;
709709
} else if (arg < 0.0) {
710-
*st = Status::Invalid("domain error");
710+
*st = Status::Invalid("logarithm of negative number");
711711
return arg;
712712
}
713713
return std::log(arg);
@@ -732,10 +732,10 @@ struct Log10Checked {
732732
static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg, Status* st) {
733733
static_assert(std::is_same<T, Arg>::value, "");
734734
if (arg == 0) {
735-
*st = Status::Invalid("divide by zero");
735+
*st = Status::Invalid("logarithm of zero");
736736
return arg;
737737
} else if (arg < 0) {
738-
*st = Status::Invalid("domain error");
738+
*st = Status::Invalid("logarithm of negative number");
739739
return arg;
740740
}
741741
return std::log10(arg);
@@ -760,10 +760,10 @@ struct Log2Checked {
760760
static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg, Status* st) {
761761
static_assert(std::is_same<T, Arg>::value, "");
762762
if (arg == 0.0) {
763-
*st = Status::Invalid("divide by zero");
763+
*st = Status::Invalid("logarithm of zero");
764764
return arg;
765765
} else if (arg < 0.0) {
766-
*st = Status::Invalid("domain error");
766+
*st = Status::Invalid("logarithm of negative number");
767767
return arg;
768768
}
769769
return std::log2(arg);
@@ -788,10 +788,10 @@ struct Log1pChecked {
788788
static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg, Status* st) {
789789
static_assert(std::is_same<T, Arg>::value, "");
790790
if (arg == -1) {
791-
*st = Status::Invalid("divide by zero");
791+
*st = Status::Invalid("logarithm of zero");
792792
return arg;
793793
} else if (arg < -1) {
794-
*st = Status::Invalid("domain error");
794+
*st = Status::Invalid("logarithm of negative number");
795795
return arg;
796796
}
797797
return std::log1p(arg);
@@ -1409,53 +1409,53 @@ const FunctionDoc atan2_doc{
14091409
{"y", "x"}};
14101410

14111411
const FunctionDoc ln_doc{
1412-
"Take natural log of arguments element-wise",
1412+
"Compute natural log of arguments element-wise",
14131413
("Non-positive values return -inf or NaN. Null values return null.\n"
14141414
"Use function \"ln_checked\" if you want non-positive values to raise an error."),
14151415
{"x"}};
14161416

14171417
const FunctionDoc ln_checked_doc{
1418-
"Take natural log of arguments element-wise",
1418+
"Compute natural log of arguments element-wise",
14191419
("Non-positive values return -inf or NaN. Null values return null.\n"
14201420
"Use function \"ln\" if you want non-positive values to return "
14211421
"-inf or NaN."),
14221422
{"x"}};
14231423

14241424
const FunctionDoc log10_doc{
1425-
"Take log base 10 of arguments element-wise",
1425+
"Compute log base 10 of arguments element-wise",
14261426
("Non-positive values return -inf or NaN. Null values return null.\n"
14271427
"Use function \"log10_checked\" if you want non-positive values to raise an error."),
14281428
{"x"}};
14291429

14301430
const FunctionDoc log10_checked_doc{
1431-
"Take log base 10 of arguments element-wise",
1431+
"Compute log base 10 of arguments element-wise",
14321432
("Non-positive values return -inf or NaN. Null values return null.\n"
14331433
"Use function \"log10\" if you want non-positive values to return "
14341434
"-inf or NaN."),
14351435
{"x"}};
14361436

14371437
const FunctionDoc log2_doc{
1438-
"Take log base 2 of arguments element-wise",
1438+
"Compute log base 2 of arguments element-wise",
14391439
("Non-positive values return -inf or NaN. Null values return null.\n"
14401440
"Use function \"log2_checked\" if you want non-positive values to raise an error."),
14411441
{"x"}};
14421442

14431443
const FunctionDoc log2_checked_doc{
1444-
"Take log base 2 of arguments element-wise",
1444+
"Compute log base 2 of arguments element-wise",
14451445
("Non-positive values return -inf or NaN. Null values return null.\n"
14461446
"Use function \"log2\" if you want non-positive values to return "
14471447
"-inf or NaN."),
14481448
{"x"}};
14491449

14501450
const FunctionDoc log1p_doc{
1451-
"Take natural log of (1+x) element-wise",
1451+
"Compute natural log of (1+x) element-wise",
14521452
("Values <= -1 return -inf or NaN. Null values return null.\n"
14531453
"This function may be more precise than log(1 + x) for x close to zero."
14541454
"Use function \"log1p_checked\" if you want non-positive values to raise an error."),
14551455
{"x"}};
14561456

14571457
const FunctionDoc log1p_checked_doc{
1458-
"Take natural log of (1+x) element-wise",
1458+
"Compute natural log of (1+x) element-wise",
14591459
("Values <= -1 return -inf or NaN. Null values return null.\n"
14601460
"This function may be more precise than log(1 + x) for x close to zero."
14611461
"Use function \"log1p\" if you want non-positive values to return "

‎cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1827,7 +1827,7 @@ TYPED_TEST(TestUnaryArithmeticFloating, Log) {
18271827
this->SetNansEqual(true);
18281828
for (auto check_overflow : {false, true}) {
18291829
this->SetOverflowCheck(check_overflow);
1830-
this->AssertUnaryOp(Ln, "[1, 2.7182818284590452354, null, NaN, Inf]",
1830+
this->AssertUnaryOp(Ln, "[1, 2.718281828459045, null, NaN, Inf]",
18311831
"[0, 1, null, NaN, Inf]");
18321832
// N.B. min() for float types is smallest normal number > 0
18331833
this->AssertUnaryOp(Ln, std::numeric_limits<CType>::min(),
@@ -1851,26 +1851,32 @@ TYPED_TEST(TestUnaryArithmeticFloating, Log) {
18511851
this->AssertUnaryOp(Log1p, std::numeric_limits<CType>::max(),
18521852
std::log1p(std::numeric_limits<CType>::max()));
18531853
}
1854-
this->AssertUnaryOpRaises(Ln, "[0]", "divide by zero");
1855-
this->AssertUnaryOpRaises(Ln, "[-1]", "domain error");
1856-
this->AssertUnaryOpRaises(Ln, "[-Inf]", "domain error");
1854+
this->SetOverflowCheck(false);
1855+
this->AssertUnaryOp(Ln, "[-Inf, -1, 0, Inf]", "[NaN, NaN, -Inf, Inf]");
1856+
this->AssertUnaryOp(Log10, "[-Inf, -1, 0, Inf]", "[NaN, NaN, -Inf, Inf]");
1857+
this->AssertUnaryOp(Log2, "[-Inf, -1, 0, Inf]", "[NaN, NaN, -Inf, Inf]");
1858+
this->AssertUnaryOp(Log1p, "[-Inf, -2, -1, Inf]", "[NaN, NaN, -Inf, Inf]");
1859+
this->SetOverflowCheck(true);
1860+
this->AssertUnaryOpRaises(Ln, "[0]", "logarithm of zero");
1861+
this->AssertUnaryOpRaises(Ln, "[-1]", "logarithm of negative number");
1862+
this->AssertUnaryOpRaises(Ln, "[-Inf]", "logarithm of negative number");
18571863
this->AssertUnaryOpRaises(Ln, MakeArray(std::numeric_limits<CType>::lowest()),
1858-
"domain error");
1859-
this->AssertUnaryOpRaises(Log10, "[0]", "divide by zero");
1860-
this->AssertUnaryOpRaises(Log10, "[-1]", "domain error");
1861-
this->AssertUnaryOpRaises(Log10, "[-Inf]", "domain error");
1864+
"logarithm of negative number");
1865+
this->AssertUnaryOpRaises(Log10, "[0]", "logarithm of zero");
1866+
this->AssertUnaryOpRaises(Log10, "[-1]", "logarithm of negative number");
1867+
this->AssertUnaryOpRaises(Log10, "[-Inf]", "logarithm of negative number");
18621868
this->AssertUnaryOpRaises(Log10, MakeArray(std::numeric_limits<CType>::lowest()),
1863-
"domain error");
1864-
this->AssertUnaryOpRaises(Log2, "[0]", "divide by zero");
1865-
this->AssertUnaryOpRaises(Log2, "[-1]", "domain error");
1866-
this->AssertUnaryOpRaises(Log2, "[-Inf]", "domain error");
1869+
"logarithm of negative number");
1870+
this->AssertUnaryOpRaises(Log2, "[0]", "logarithm of zero");
1871+
this->AssertUnaryOpRaises(Log2, "[-1]", "logarithm of negative number");
1872+
this->AssertUnaryOpRaises(Log2, "[-Inf]", "logarithm of negative number");
18671873
this->AssertUnaryOpRaises(Log2, MakeArray(std::numeric_limits<CType>::lowest()),
1868-
"domain error");
1869-
this->AssertUnaryOpRaises(Log1p, "[-1]", "divide by zero");
1870-
this->AssertUnaryOpRaises(Log1p, "[-2]", "domain error");
1871-
this->AssertUnaryOpRaises(Log1p, "[-Inf]", "domain error");
1874+
"logarithm of negative number");
1875+
this->AssertUnaryOpRaises(Log1p, "[-1]", "logarithm of zero");
1876+
this->AssertUnaryOpRaises(Log1p, "[-2]", "logarithm of negative number");
1877+
this->AssertUnaryOpRaises(Log1p, "[-Inf]", "logarithm of negative number");
18721878
this->AssertUnaryOpRaises(Log1p, MakeArray(std::numeric_limits<CType>::lowest()),
1873-
"domain error");
1879+
"logarithm of negative number");
18741880
}
18751881

18761882
} // namespace compute

0 commit comments

Comments
 (0)