[Rd] ifelse() woes ... can we agree on a ifelse2() ?

[Suharto Anggono Suharto Anggono]

Interspersed below.
--------------------------------------------


 Subject: Re: ifelse() woes ... can we agree on a ifelse2() ?
 To: R-devel at lists.R-project.org
 Date: Sunday, 27 November, 2016, 12:14 AM
 
On current 'ifelse' code in R:
...
* If 'test' is a factor, doing
storage.mode(test) <- "logical"
is not appropriate, but is.atomic(test) returns TRUE. Maybe use
if(!is.object(test))
instead of
if(is.atomic(test)) .
===================================
I now see that, for 'test' that is atomic and has "class" attribute, with current 'ifelse' code, changing
if(is.atomic(test))
to
if(!is.object(test))
removes class of 'test' and makes the result doesn't have class of 'test', which is not according to the documentation. The documentation of 'ifelse' says that the value is "A vector of the same length and attributes (including dimensions and "class") as 'test' ...".
===================================


function(test, yes, no, NA. = NA) {
    if(!is.logical(test))
        test <- if(isS4(test)) methods::as(test, "logical") else as.logical(test)
    n <- length(test)
    n.yes <- length(yes); n.no <- length(no)
    if (n.yes != n) {
        if (n.no == n) {  # swap yes <-> no
            test <- !test
            ans <- yes; yes <- no; no <- ans
            n.no <- n.yes
        } else yes <- yes[rep_len(seq_len(n.yes), n)]
    }
    ans <- yes
    if (n.no == 1L)
        ans[!test] <- no
    else
        ans[!test & !is.na(test)] <- no[
            if (n.no == n) !test & !is.na(test)
            else rep_len(seq_len(n.no), n)[!test & !is.na(test)]]
    stopifnot(length(NA.) == 1L)
    ans[is.na(test)] <- NA.
    ans
}

===================================
For data frame, indexing by logical matrix is different from indexing by logical vector.
Because there is an example like that, I think that it is better to remove
if(!is.logical(test))
in the function definition above, making 'as.logical' also applied to 'test' of mode "logical", stripping attributes. Doing so makes sure that 'test' is a plain logical vector, so that indexing is compatible with 'length'.