pitrou commented on code in PR #49290:
URL: https://github.com/apache/arrow/pull/49290#discussion_r3414289162
##########
cpp/src/arrow/compare.h:
##########
@@ -63,26 +65,49 @@ class EqualOptions {
///
/// This option only affects the Equals methods
/// and has no effect on ApproxEquals methods.
- bool use_atol() const { return use_atol_; }
+ /// \deprecated Deprecated in 24.0.0. Use arrow::EqualOptions::atol instead
+ ARROW_DEPRECATED("Deprecated in 24.0.0. Use arrow::EqualOptions::atol
instead")
+ bool use_atol() const { return atol_.has_value(); }
/// Return a new EqualOptions object with the "use_atol" property changed.
+ /// \deprecated Deprecated in 24.0.0. Use arrow::EqualOptions::atol instead
+ ARROW_DEPRECATED("Deprecated in 24.0.0. Use arrow::EqualOptions::atol
instead")
EqualOptions use_atol(bool v) const {
auto res = EqualOptions(*this);
- res.use_atol_ = v;
+ if (v) {
+ res.atol_ = atol_.value_or(kDefaultAbsoluteTolerance);
+ } else {
+ res.atol_ = kDefaultAbsoluteTolerance;
Review Comment:
Why not `std::nullopt` here?
##########
cpp/src/arrow/compare.h:
##########
@@ -63,26 +65,49 @@ class EqualOptions {
///
/// This option only affects the Equals methods
/// and has no effect on ApproxEquals methods.
- bool use_atol() const { return use_atol_; }
+ /// \deprecated Deprecated in 24.0.0. Use arrow::EqualOptions::atol instead
+ ARROW_DEPRECATED("Deprecated in 24.0.0. Use arrow::EqualOptions::atol
instead")
Review Comment:
Note that this should say 25.0.0 now, not 24.0.0 :-)
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -2199,11 +2199,10 @@ void CheckFloatApproxEqualsWithAtol() {
auto options = EqualOptions::Defaults().atol(0.2);
ASSERT_FALSE(a->Equals(b));
- ASSERT_TRUE(a->Equals(b, options.use_atol(true)));
ASSERT_TRUE(a->ApproxEquals(b, options));
- ASSERT_FALSE(a->RangeEquals(0, 1, 0, b, options));
- ASSERT_TRUE(a->RangeEquals(0, 1, 0, b, options.use_atol(true)));
Review Comment:
Same for this one.
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -2199,11 +2199,10 @@ void CheckFloatApproxEqualsWithAtol() {
auto options = EqualOptions::Defaults().atol(0.2);
ASSERT_FALSE(a->Equals(b));
- ASSERT_TRUE(a->Equals(b, options.use_atol(true)));
Review Comment:
I think we can keep this test while protecting it with
`ARROW_SUPPRESS_DEPRECATION_WARNING`. Something like:
```c++
ARROW_SUPPRESS_DEPRECATION_WARNING
ASSERT_TRUE(a->Equals(b, options.use_atol(true)));
ARROW_UNSUPPRESS_DEPRECATION_WARNING
```
--
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]
