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_setequal doesn't work on lists (document?) #750

Closed
MichaelChirico opened this issue Apr 27, 2018 · 3 comments
Closed

expect_setequal doesn't work on lists (document?) #750

MichaelChirico opened this issue Apr 27, 2018 · 3 comments

Comments

@MichaelChirico
Copy link
Contributor

@MichaelChirico MichaelChirico commented Apr 27, 2018

I'd expect the following test to pass: expect_setequal(list(a = 1:4, b = 4:1), list(b = 4:1, a = 1:4)), but it fails because the inputs are sorted with sort.int, which doesn't know how to handle a list.

I was attempting to replace a test of the form expect_equal(my_list[order(names(my_list))], list(a = 1:4, b = 4:1)).

If there's disagreement about whether these objects should be considered setequal, at the very least we need to document the acceptable types (atomic?) to be passed to object as ?expect_setequal only says:

object object to test
@kevinykuo
Copy link

@kevinykuo kevinykuo commented Aug 27, 2018

This feels like a bug; one would expect expect_setequal() to pass if setequal is TRUE.

@kevinushey
Copy link
Collaborator

@kevinushey kevinushey commented Dec 14, 2018

I agree this would be nice to have.

@jimhester jimhester added the bug label Dec 14, 2018
kevinushey added a commit to kevinushey/testthat that referenced this issue Dec 14, 2018
@hadley
Copy link
Member

@hadley hadley commented Mar 28, 2019

Set equality ignores names:

setequal(
  list(a = 1:4, b = 4:1), 
  list(d = 4:1, e = 1:4)
)
#> [1] TRUE

Created on 2019-03-28 by the reprex package (v0.2.1.9000)

But maybe expect_setequal() should use an implementation closer to setequal() and avoiding sorting. We could using something like all(vec_in(x, y)) && all(vec_in(y, x)). The error reporting will be a bit trickier though.

@hadley hadley added this to the testthat 2.1.0 milestone Apr 1, 2019
@hadley hadley closed this in a737622 Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants