false equivalence in vroom/tests/testthat/test-factor.R (due to utils::all.equal not testing factors well)

[BillDunlap]

Some of the tests in vroom/tests/testthat/test-factor.R pass in R and fail in TERR even though the objects created are identical in R and TERR. I found that this was due to R's util::all.equal() not being strict enough when comparing factors. Here is simplified version of one of the tests.

t <- tibble::tibble(X1 = factor(c(NA, "b", "a"), levels = c("a", "b", NA), exclude = NULL))
v <- vroom::vroom("NA\nb\na\n", col_names = FALSE, col_types = list(X1 = col_factor(levels = c("a", "b", NA))), delim="\n")                                                                                                                      all.equal(t$X1, v$X1)
#R:  [1] TRUE
#TERR: [1] "missing value mismatches: 1"

R and TERR agree that there are no NAs in t$X1 and that the first element of v$X1 is an NA:

> is.na(t$X1)
[1] FALSE FALSE FALSE
> is.na(v$X1)
[1]  TRUE FALSE FALSE
They both produce the same t$X1 and v$X1:
> dput(t$X1)
structure(3:1, .Label = c("a", "b", NA), class = "factor")
> dput(v$X1)
structure(c(NA, 2L, 1L), .Label = c("a", "b", NA), class = "factor")

TERR's all.equal(x,y) for factors compares is.na(x) and is.na(y) and apparently R's does not (https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17897). I think this difference in behavior between tibble and vroom is significant enough that it should be fixed.

[mmaechler]

I've fixed this in R yesterday, currently only for R-devel (to be released as R x.y.0 ca. April 2021)

[jimhester]

To close the loop vroom after @mmaechler's change the vroom tests are now failing on R-devel.

  ── 1. Failure: NAs included in levels if desired (@test-factor.R#55) ──────────
     vroom(content, delim = delim, ...) not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 2. Failure: NAs included in levels if desired (@test-factor.R#55) ──────────
     vroom(content, delim = delim, ...) not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 3. Failure: NAs included in levels if desired (@test-factor.R#55) ──────────
     `res` not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 4. Failure: NAs included in levels if desired (@test-factor.R#55) ──────────
     `res` not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 5. Failure: NAs included in levels if desired (@test-factor.R#60) ──────────
     vroom(content, delim = delim, ...) not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 6. Failure: NAs included in levels if desired (@test-factor.R#60) ──────────
     vroom(content, delim = delim, ...) not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 7. Failure: NAs included in levels if desired (@test-factor.R#60) ──────────
     `res` not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 8. Failure: NAs included in levels if desired (@test-factor.R#60) ──────────
     `res` not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 9. Failure: NAs included in levels if desired (@test-factor.R#70) ──────────
     vroom(content, delim = delim, ...) not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 10. Failure: NAs included in levels if desired (@test-factor.R#70) ─────────
     vroom(content, delim = delim, ...) not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 11. Failure: NAs included in levels if desired (@test-factor.R#70) ─────────
     `res` not equal to `equals`.
     Component "X1": 1, NA mismatch
    
     ── 12. Failure: NAs included in levels if desired (@test-factor.R#70) ─────────
     `res` not equal to `equals`.
     Component "X1": 1, NA mismatch

I will fix this and submit a new version of vroom in the near future.

Thank you both!

[BillDunlap]

Note that this is vroom's issue #262: #262.

…

On Thu, Sep 17, 2020 at 6:21 AM Jim Hester ***@***.***> wrote: To close the loop vroom after @mmaechler <https://github.com/mmaechler>'s change the vroom tests are now failing on R-devel. ── 1. Failure: NAs included in levels if desired ***@***.***#55) ────────── vroom(content, delim = delim, ...) not equal to `equals`. Component "X1": 1, NA mismatch ── 2. Failure: NAs included in levels if desired ***@***.***#55) ────────── vroom(content, delim = delim, ...) not equal to `equals`. Component "X1": 1, NA mismatch ── 3. Failure: NAs included in levels if desired ***@***.***#55) ────────── `res` not equal to `equals`. Component "X1": 1, NA mismatch ── 4. Failure: NAs included in levels if desired ***@***.***#55) ────────── `res` not equal to `equals`. Component "X1": 1, NA mismatch ── 5. Failure: NAs included in levels if desired ***@***.***#60) ────────── vroom(content, delim = delim, ...) not equal to `equals`. Component "X1": 1, NA mismatch ── 6. Failure: NAs included in levels if desired ***@***.***#60) ────────── vroom(content, delim = delim, ...) not equal to `equals`. Component "X1": 1, NA mismatch ── 7. Failure: NAs included in levels if desired ***@***.***#60) ────────── `res` not equal to `equals`. Component "X1": 1, NA mismatch ── 8. Failure: NAs included in levels if desired ***@***.***#60) ────────── `res` not equal to `equals`. Component "X1": 1, NA mismatch ── 9. Failure: NAs included in levels if desired ***@***.***#70) ────────── vroom(content, delim = delim, ...) not equal to `equals`. Component "X1": 1, NA mismatch ── 10. Failure: NAs included in levels if desired ***@***.***#70) ───────── vroom(content, delim = delim, ...) not equal to `equals`. Component "X1": 1, NA mismatch ── 11. Failure: NAs included in levels if desired ***@***.***#70) ───────── `res` not equal to `equals`. Component "X1": 1, NA mismatch ── 12. Failure: NAs included in levels if desired ***@***.***#70) ───────── `res` not equal to `equals`. Component "X1": 1, NA mismatch I will fix this and submit a new version of vroom in the near future. Thank you both! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#262 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACY5MBJ5XEDADTFVD5GUIW3SGIEMRANCNFSM4QMGIPIA> .