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

expect_equals(a,b,0.01) silently ignores 0.01 from v0.10.1 #294

Closed
mattdowle opened this issue Sep 10, 2015 · 1 comment
Closed

expect_equals(a,b,0.01) silently ignores 0.01 from v0.10.1 #294

mattdowle opened this issue Sep 10, 2015 · 1 comment
Milestone

Comments

@mattdowle
Copy link

v0.9.1 : expected behaviour

> expect_equal(415, 416, 0.01)
> expect_equal(415, 416, tolerance=0.01)

v0.10.0 May 2015, currently on CRAN

> expect_equal(415, 416, 0.01)   # 0.01 silently not passed on to all.equal, apparently
Error: 415 not equal to 416
416 - 415 == 1
> expect_equal(415, 416, tolerance=0.01)   # tolerance= now required.

We all know we should always name our arguments. Anyone who hasn't may now be bitten by this when they upgrade. It's the silent apparent ignoring of the 3rd argument that's confusing. ?expect_equals contains ... = other values passed to all.equal and when you then look at ?all.equal it seems reasonable to assume the 3rd argument matches up with the 3rd argument of all.equal (i.e. tolerance).

A few solutions I can think of are i) restoring previous behaviour, ii) catch and warn/error about any unnamed arguments or iii) add to ?expect_equals that tolerance must now be explicitly named.

@hadley
Copy link
Member

hadley commented Sep 10, 2015

Hmmmm, I think the fix is to change the order of the compare.numeric arguments. Is currently (x, y, max_diffs = 10, ...), but I think it needs to be (x, y, ..., max_diffs = 10)

@hadley hadley modified the milestone: 0.11.0 Sep 29, 2015
@hadley hadley closed this as completed in 87895be Sep 29, 2015
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