Skip to content

Use of all/any

5 messages · Brian Ripley, Martin Maechler, Duncan Murdoch +1 more

Original
Brian Ripley
# 
all/any coerce their arguments to logical (if possible).  I've added a 
warning in R-devel if coercion is from something other than integer.

This arose because it is easy to make a slip and write all(X) > 0 rather 
than all(X > 0): thanks to Bill Dunlap for bringing that to my attention.
However, it has been useful in detecting quite a few other things:

- indices which had been made double where integer was intended. One 
example from predict.lm was

                 iipiv[ii == 0] <- 0

which was intended to be

                 iipiv[ii == 0L] <- 0L

- uses of lapply where sapply was intended.  Examples are of the form

 	all(lapply(z, is.numeric))

which is applying all() to a list.  One might worry that

 	sapply(z, is.numeric)

will return a list if length(z) == 0 (which it does) and so all() would 
warn, but that is covered by another change, to ignore all length-zero 
arguments (and so avoid the cost of coercion to logical(0)).


I decided not to warn on integer as it is so common.  But at least some of 
these are thinkos.  For example, constructions like

 	all(grep(pattern, x))

occurred scores of times in the R sources.  Since the value of grep() is 
an integer vector of positive indices, this is equivalent to

 	length(grep(pattern, x)) > 0

and when used in a if() condition the '> 0' is not needed.


Some warnings are common from other packages: one is

Warning in any(textLocations) :
   coercing argument of type 'double' to logical

from lattice (and Deepayan Sarkar will fix that shortly).  Quite a few 
others looked familiar but are the result of package authors copying code 
from base R or other packages: if you do that you do need to copy the 
bugfixes too.
Martin Maechler
# 
BDR> all/any coerce their arguments to logical (if
    BDR> possible).  I've added a warning in R-devel if coercion
    BDR> is from something other than integer.

    BDR> This arose because it is easy to make a slip and write
    BDR> all(X) > 0 rather than all(X > 0): thanks to Bill
    BDR> Dunlap for bringing that to my attention.  


    BDR> However, it has been useful in detecting quite a few other things:

    BDR> - indices which had been made double where integer was
    BDR> intended. One example from predict.lm was

    BDR>                  iipiv[ii == 0] <- 0

    BDR> which was intended to be

    BDR>                  iipiv[ii == 0L] <- 0L

Hmm....  Do we really want to generate warnings for such small
inefficiencies?
I'm very happy that we've introduced   <n>L integer notation, and
as a subtle programmer, I'm making use of it gladly --- but
still not always, just for code beauty reasons ("0" reads better).

On the other hand, I don't think the casual R / S programmer
should get warnings; after all, S and R  are not C on purpose.

Apropos Bill Dunlap's note:  Do newer versions of S-plus warn?
At least up to 6.2.2, I'm pretty sure no S version has warned
about
	X <- c(0.1, pi)
	all(X) > 0.5

In spite, of the buglets of you have revealed, mentioned below,
currently, I'd still tend to only warn for coercion from
non-numeric, but not from double.

In this context, I have thought again of using *levels* of
warnings, configurable via options(), and we could activate more
stringent warnings when "R CMD check"ing than per default.

Actually, we already have a simple form of that (with I think message()),
and also with the way the 'codetools' ``warnings'' are treated
by 'R CMD check'.
For my taste and "S language feeling", such a	
    'double -> logical coercion warning'
is somewhat similar in sprit to some of the codetools warnings.

Martin



    BDR> - uses of lapply where sapply was intended.  Examples
    BDR> are of the form

    BDR>  	all(lapply(z, is.numeric))

    BDR> which is applying all() to a list.  One might worry
    BDR> that

    BDR>  	sapply(z, is.numeric)

    BDR> will return a list if length(z) == 0 (which it does)
    BDR> and so all() would warn, but that is covered by another
    BDR> change, to ignore all length-zero arguments (and so
    BDR> avoid the cost of coercion to logical(0)).


    BDR> I decided not to warn on integer as it is so common.
    BDR> But at least some of these are thinkos.  For example,
    BDR> constructions like

    BDR>  	all(grep(pattern, x))

    BDR> occurred scores of times in the R sources.  Since the
    BDR> value of grep() is an integer vector of positive
    BDR> indices, this is equivalent to

    BDR>  	length(grep(pattern, x)) > 0

    BDR> and when used in a if() condition the '> 0' is not
    BDR> needed.


    BDR> Some warnings are common from other packages: one is

    BDR> Warning in any(textLocations) : coercing argument of
    BDR> type 'double' to logical

    BDR> from lattice (and Deepayan Sarkar will fix that
    BDR> shortly).  Quite a few others looked familiar but are
    BDR> the result of package authors copying code from base R
    BDR> or other packages: if you do that you do need to copy
    BDR> the bugfixes too.
Duncan Murdoch
#
On 10/26/2007 12:18 PM, Martin Maechler wrote:
I don't know whether S warns about that, but isn't it clear that it 
should generate a warning?  That's almost certainly a typo for

  all(X > 0.5)

If someone really wanted to do what all(X) > 0.5 says, then they should 
code it clearly as

  all(X != 0)

and not try to win an obfuscated code contest by coding it in the 
original way.

Duncan Murdoch
Bill Dunlap
#
On Fri, 26 Oct 2007, Martin Maechler wrote:

            

      
      
      
    

        
Hi Martin,

No, it doesn't warn.  We had a user who ran into a bug
in another function that came from this sort of thing
so I wrote some code that examined parse trees for
expresions of the form
    <comparison operator>
        <call to any or all>
        <anything>
or
    <comparison operator>
        <anything>
        <call to any or all>
and ran it over all our source code.  Out of curiousity
I also ran it over the current R source and an out-of-date
copy of the package source code from CRAN and that is
where I ran across the problem in polr() (and lots of instances
in packages, although they seemed to be clustered).

Now I have a question/complaint about doing this.  In Splus
I looked for this pattern with the following code
   isComparisonOfAnyOrAll <- function(expr)
      isCallTo(expr, c("<", ">", "<=", ">=", "==")) &&
        (isCallTo(expr[[2]], c("any", "all")) ||
         isCallTo(expr[[3]], c("any", "all")))
where isCallTo is
   isCallTo <- function(expr, functionName, numberArgs = NULL)
   {
        # return TRUE if expr is a call to with one of the functions
	# listed in functionName.  If numberArgs is non-NULL, it should
	# be a nonnegative integer giving the required number of arguments
	# in the call
        if(class(expr) == "call with ...") {
                # e.g., Quote(foo(x, ..., value = TRUE))
                # This class is only in Splus
                if(!is.null(numberArgs)) {
                        warning("call has ... in argument list, numberArgs will count all ... arguments as 1 argument"
                                )
                }
                expr <- expr[[1]]
        }
        if(length(functionName) == 1) {
                retval <- class(expr) == "call" && identical(expr[[1]], as.name(
                        functionName)) && (is.null(numberArgs) || numberArgs ==
                        length(expr) - 1)
        }
        else {
                retval <- class(expr) == "call" && is.name(expr[[1]]) &&
                        is.element(as.character(expr[[1]]), functionName) &&
                        (is.null(numberArgs) || numberArgs == length(expr) -
                        1)
        }
        retval
   }
This code works in Splus and R.  E.g.,
   > isComparisonOfAnyOrAll(Quote(any(x)<0))
   [1] TRUE
   > isComparisonOfAnyOrAll(Quote(any(x<0)))
   [1] FALSE

In Splus I use
   rapply(expr, classes="call",
     f=function(x)if(isComparisonOfAnyOrAll(x))deparseText(x))
to rattle down an an expression tree looking for this pattern.
However's R's rapply won't let me do that because
it insists its input be a function instead of being of
recursive type.  Its help file says it evaluates the arguments
to f() even if they are expressions, and that may contribute
to problems.  The Splus rapply accepts any recursive type and it does not
evaluate the subtrees that it hands to f().

E.g., running the same input into R and Splus and labelling
the output lines 'Splus:' and 'R   :', we get
  RS> rapply(function(x)log(x+1), f = function(expr) if (is.name(expr)) as.character(expr)) # all.names()
  Splus: [1] "log" "+"   "x"
  R    : Error in rapply(function(x) log(x + 1), f = function(expr) if (is.name(expr)) as.character(expr)) :
  R    :  'object' must be a list
If I get around the "'object' must be a list' problem by wrapping
the input in a list then I run into the evalution problem

Does R have an rapply-like function that works like Splus's?

Are the R parse tree classes sufficiently different from lists
that we cannot expect the above to work?

In Splus I've used rapply quite productively to find patterns
in parse trees and then change the code.  E.g., to change all
calls of the form
    log(x, base)
to
    logb(x, base)
but not change calls of the form log(x) you can do
    > changeLogCalls<-function(func) {
        rapply(func, classes="call", how="replace",
            function(expr){
                if(isCallTo(expr,"log",2)) expr[[1]] <- as.name("logb")
                expr
            })
    }
    }
    > changeLogCalls(function(x)log(x,2)/log(x))
    function(x)
    logb(x, 2)/log(x)

I suspect I should be looking in codetools for this sort of
thing.

----------------------------------------------------------------------------
Bill Dunlap
Insightful Corporation
bill at insightful dot com

 "All statements in this message represent the opinions of the author and do
 not necessarily reflect Insightful Corporation policy or position."

  
  


  


      

    

    

      
      

        


  

        

      Bill Dunlap
    

    

      
      #
    

  


  

      
On Fri, 26 Oct 2007, Bill Dunlap wrote:

        

            

      
      
      
    

        
oops, "a list", not "function"

            

      
      
      
    

        
----------------------------------------------------------------------------
Bill Dunlap
Insightful Corporation
bill at insightful dot com
360-428-8146

 "All statements in this message represent the opinions of the author and do
 not necessarily reflect Insightful Corporation policy or position."