Flip bit64.promoteInteger64ToCharacter to TRUE by default

[MichaelChirico]

@hcirellu I am not sure we have the right behavior here, WDYT?

str(c(A=integer(), B=as.factor(x32)))
#  Named int [1:10] 1 2 3 4 5 6 7 8 9 10
#  - attr(*, "names")= chr [1:10] "B1" "B2" "B3" "B4" ...
str(c(A=integer64(), B=as.factor(x32)))
#  Named chr [1:10] "1" "2" "3" "4" "5" "6" "7" "8" "9" "10"
#  - attr(*, "names")= chr [1:10] "B1" "B2" "B3" "B4" ...

Shouldn't the result be a factor()?

We currently have 6 failing tests after the flip due to this at root AIUI.

[hcirellu]

Yes the result should be factor and not character.

[hcirellu]

Wait, no - it should be integer64, since c(A=1L, B=as.factor("a")) is integer.

[hcirellu]

You may change target_class to the following:

target_class = function(x, recursive=FALSE, POSIXltAsCharacter=FALSE, forSetOps=FALSE) {
  classes = getClassesOfElements(x, recursive=isTRUE(recursive))

  if ("character" %in% classes || (isTRUE(forSetOps) && any(c("factor", "ordered") %in% classes))) {
    # TODO(#44): next Release: Add warning for "TRUE"; subsequent Release: change from warning to error; subsequent Release: remove option and promote_to_char
    if (isTRUE(getOption("bit64.promoteInteger64ToCharacter", TRUE))) return("character")
  }
  if ("POSIXlt" %in% classes && isTRUE(POSIXltAsCharacter)) return("character")
  if ("complex" %in% classes) return("complex")
  "integer64"
}

and add forSetOps=TRUE in the setops functions (e.g. in union(): target_class = target_class(list(x, y), forSetOps=TRUE)), so that for these functions the result of union(as.integer64(1L), as.factor("a")) is consistent to union(1L, as.factor("a")).

That would leave 6 failing tests in test_setops64 for setdiff and is.element that needs to be looked into.

[hcirellu]

I hope it is ok that I committed to your branch directly.

[hcirellu]

I am surprised by the white spaces around argument assignments, e.g. dimnames = list(NULL, c("a", "b")) instead of dimnames=list(NULL, c("a", "b")). Are you changing the style?

Why are you hesitating to merge this PR?