R devel - Nov 2016 - ifelse() woes ... can we agree on a ifelse2() ?

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'.