Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

document how waldo::compare and all.equal differ for relative numeric comparison #211

Open
jaganmn opened this issue Nov 8, 2024 · 2 comments

Comments

@jaganmn
Copy link

jaganmn commented Nov 8, 2024

Perhaps this issue is known or has been reported before - in that case, apologies.

all.equal uses its first argument to compute the denominator of the relative difference. waldo::compare uses its second argument, leading to differences in behaviour as here:

> x <- c( 0.077172645966023900, -0.000266267947825388, -0.01796820303767930,
+        -0.000266267947825388,  0.026773667457362100,  0.00885332514392564,
+        -0.017968203037679300,  0.008853325143925640,  0.03937164346237700)
> y <- c(0.04, 0, 0, 0, 0.04, 0, 0, 0, 0.04)
> tol <- 0.02
> all.equal(x, y, tolerance = tol)
[1] "Mean relative difference: 0.5326904"
> all.equal(y, x, tolerance = tol)
[1] TRUE
> waldo::compare(x, y, tolerance = tol)
✔ No differences
> waldo::compare(y, x, tolerance = tol)
`old`: 0.040  0.000  0.000  0.000 0.040 0.000  0.000 0.000 0.040
`new`: 0.077 -0.000 -0.018 -0.000 0.027 0.009 -0.018 0.009 0.039
>

help("compare", package="waldo") and help("expect_equal", package="testthat") don't claim that they are drop-in replacements for all.equal(...) and stopifnot(all.equal(...)), but that seems to be a prevailing belief. The situation would be improved with clearer documentation about the correspondence of the arguments.

@jaganmn
Copy link
Author

jaganmn commented Nov 8, 2024

A corollary is that testthat::expect_equal(x, y, tolerance = tol) can fail under edition 2 and pass under edition 3. In that case, with the arguments reversed, the test would pass under edition 2 and fail under edition 3.

@MichaelChirico
Copy link
Contributor

MichaelChirico commented Nov 8, 2024

Hmm I thought of this as a difference in signatures since testthat::expect_equal() does object,expected whereas all.equal() does target,current which (to me) maps to actual,expected vs. expected,actual.

But if we look carefully at waldo::compare()'s signature directly, it gives the same impression as all.equal():

waldo::compare
function (x, y, ..., x_arg = "old", y_arg = "new"...)

old ~~ expected
new ~~ actual

The terminology is admittedly all over the place as we can see :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants