mllg / checkmate

Fast and versatile argument checks
https://mllg.github.io/checkmate/
Other
261 stars 30 forks source link

feat: add check for permutation #230

Closed sebffischer closed 1 year ago

sebffischer commented 2 years ago

The only thing I am unhappy with is the any(x != y), there should be a faster way to test this. Any suggestions? But I could not quickly find a function that does that. At least identical might be tricky with factors. Maybe some version of all.equal?

mllg commented 2 years ago

A more general question: What is the intended behaviour if x or y have duplicated values? I'm a bit confused by the comment

   # now this is checkSetEqual(..., ordered = TRUE) with additional sorting.
mllg commented 2 years ago

Also, x != y will not work with missing values.

Edit: Well, at least not in all cases, e.g. any(c(FALSE,NA) != c(FALSE, NA))

sebffischer commented 2 years ago

Duplicated values are not treated special in any way or what exactly do you mean by that?

I think the comment regarding the checkSetEqual with ordered = TRUE is actually incorrect and should be removed. What I mean by that is that comparing [1, 2, 1] with [1, 1, 2] essentially turns into checking whether all [1, 1, 2] == [1, 1, 2]. This also shows how the duplicated values are being handled.

Yes, but the x != y never contains missing values (they are dropped by the sort, see the comment).

mllg commented 2 years ago

Sorting comes after the length check, so this fails?

x = c(1, NA)
y = c(1)
sebffischer commented 2 years ago

Yes. Note that we check the length twice.

  1. Before sorting: To check the lengths of the full vectors.
  2. After sorting: To check whether the same amount of NAs was removed

Your example is captured in the first length check

Edit: I mean "Yes, this returns FALSE"

mllg commented 2 years ago

Ok, got it now. I think you could clarify a bit in the documentation, but otherwise good to be merged.

sebffischer commented 2 years ago

I added some comments

sebffischer commented 1 year ago

@mllg

sebffischer commented 1 year ago

Can this be merged @mllg ?