From ripley at stats.ox.ac.uk  Thu Mar  2 09:27:28 2006
From: ripley at stats.ox.ac.uk (Prof Brian Ripley)
Date: Thu, 2 Mar 2006 08:27:28 +0000 (GMT)
Subject: [Rd] stopifnot() suggestion
In-Reply-To: <17414.41532.507637.485340@stat.math.ethz.ch>
References: <Pine.GSO.4.62.0603011507180.13700@harper.uchicago.edu>
	<440618C6.2010106@jhsph.edu>
	<Pine.GSO.4.62.0603011601370.13700@harper.uchicago.edu>
	<Pine.LNX.4.64.0603020640230.10469@gannet.stats.ox.ac.uk>
	<17414.41532.507637.485340@stat.math.ethz.ch>
Message-ID: <Pine.LNX.4.64.0603020808360.15967@gannet.stats.ox.ac.uk>

On Thu, 2 Mar 2006, Martin Maechler wrote:

>>>>>> "BDR" == Prof Brian Ripley <ripley at stats.ox.ac.uk>
>>>>>>     on Thu, 2 Mar 2006 06:45:39 +0000 (GMT) writes:
>
>    BDR> stopifnot() is not intended for use by end-users, but for tests in
>    BDR> packages.
>
> and additionally for  "function writers"  aka 'programmeRs'.
> I think we have argued that R has stopifnot() where other
> programming languages use  assert().

Languages which do not have NA, so I have never argued such.

> It can be very convenient to have one compact
>
>   stopifnot( condition_A,  condition_B,  condition_C)
>
> statement at the beginning of your function instead of
> potentially much more verbose
>
>   if(!condition_A)
>       stop("bla A bla A bla bla")
>   if(!condition_B)
>       stop("bla B bla B bla bla")
>   if(!condition_C)
>       stop("bla C bla C bla bla")
>
> where the latter *would* produce more understandable error
> messages but need much more programmer's time.
>
>    BDR> If the writers of package tests are not aware of the perils of
>    BDR> using == or != with numbers, then it is good that they get reminded.
>
>    BDR> And we do have isTRUE for use with it.
>
> indeed!
>
> I still don't see why Dan's original proposition shouldn't be
> considered for adaption.
> One of the most valid (IMO) complaints about S and R have been the
> ``uncomprehensible error messages'' that people see occasionally.
> [[Though, sometimes the error message is well understandable
>  and it's just the user's lazyness to *read* and *think* .. ]]

I find that `incomprehensible'!

> An extra is.na() check in a stopifnot() in order to produce a
> much better message seems to me well worth.

Because what should happen with NA is not well-defined.  In one version, 
it is said that stopifnot() fails if an expression is not true.  Now, if 
that said TRUE, returning NA would not be an error and stopifnot should 
give an error.  Later on we are told it is `conceptually equivalent' to 
something which behaves as it currently does.  So this hangs on exactly 
what is meant by `true'.

It seems to me that stopifnot() should probably throw an error unless the 
logical result is TRUE, but at present it is written assuming the result 
must be TRUE or FALSE (and I have just documented that).  And if you 
advocate it as the analogue of assert, then I think the behaviour needs to 
be changed.

>
> Martin
>
>
>    BDR> On Wed, 1 Mar 2006, Dan Davison wrote:
>
>    >> On Wed, 1 Mar 2006, Roger D. Peng wrote:
>    >>
>    >>> Wouldn't it be better to do something like
>    >>>
>    >>> stopifnot(all(!is.na(x)), all(!is.na(y)), x, y)
>    >>>
>    >>> rather than have stopifnot() go checking for NAs?  I agree the message is
>    >>> strange but if having non-NA values is really a condition, then why not just
>    >>> put it in the call to stopifnot()?
>    >>>
>    >>> -roger
>    >>>
>    >>
>    >> I was thinking of a fallible R user accidentally testing the truth of an
>    >> expression with NAs, rather than of a situation where you remember that
>    >> there may be missing values. For example
>    >>
>    >>> f <- function() { x <- NA ; if(x != 4) stop("x should be 4") }
>    >>> g <- function() { x <- NA ; stopifnot(x == 4) }
>    >>> f()
>    >> Error in if (x != 4) stop("x should be 4") :
>    >> missing value where TRUE/FALSE needed
>    >>> g()
>    >> Error in if (!(is.logical(r <- eval(ll[[i]])) && all(r)))
>    >> stop(paste(deparse(mc[[i +  :
>    >> missing value where TRUE/FALSE needed
>    >>
>    >> If you write the error-checking code represented by f(), you get a message
>    >> which is very helpful in correcting your error. But someone who uses
>    >> stopifnot() instead gets the output of g(). Even a user who knows the
>    >> origin of the code in the error message doesn't know which of several
>    >> stopifnot()s is responsible.
>    >>
>    >> Dan
>    >>
>    >>
>    >>
>    >>> Dan Davison wrote:
>    >>>> If an expression is passed to stopifnot() which contains missing values,
>    >>>> then the resulting error message is somewhat baffling until you are used to
>    >>>> it, e.g.
>    >>>>
>    >>>>> x <- y <- rep(TRUE, 10)
>    >>>>> y[7] <- NA
>    >>>>> stopifnot(x, y)
>    >>>> Error in if (!(is.logical(r <- eval(ll[[i]])) && all(r)))
>    >>>> stop(paste(deparse(mc[[i +  :
>    >>>> missing value where TRUE/FALSE needed
>    >>>>
>    >>>> A minor change to stopifnot() produces the following behaviour:
>    >>>>
>    >>>>> stopifnot(x, y)
>    >>>> Error in stopifnot(x, y) : y contains missing values
>    >>>>
>    >>>> My attempt at a suitable modification follows, and below that the original
>    >>>> function definition. Is a change along these lines appropriate?
>    >>>>
>    >>>> ## Altered version
>    >>>>
>    >>>> stopifnot <- function (...) {
>    >>>> n <- length(ll <- list(...))
>    >>>> if (n == 0)
>    >>>> return(invisible())
>    >>>> mc <- match.call()
>    >>>> for (i in 1:n) {
>    >>>> if(any(is.na(r <- eval(ll[[i]])))) stop(paste(deparse(mc[[i +
>    >>>> 1]])), " contains missing values")
>    >>>> if (!(is.logical(r) && all(r)))
>    >>>> stop(paste(deparse(mc[[i + 1]]), "is not TRUE"), call. =
>    >>>> FALSE)
>    >>>> }
>    >>>> }
>    >>>>
>    >>>>
>    >>>> ## from R-2.1.1/src/library/base/R/stop.R
>    >>>>
>    >>>> stopifnot <- function(...)
>    >>>> {
>    >>>> n <- length(ll <- list(...))
>    >>>> if(n == 0)
>    >>>> return(invisible())
>    >>>> mc <- match.call()
>    >>>> for(i in 1:n)
>    >>>> if(!(is.logical(r <- eval(ll[[i]])) && all(r)))
>    >>>> stop(paste(deparse(mc[[i+1]]), "is not TRUE"), call. = FALSE)
>    >>>> }
>    >>>>
>    >>>>
>    >>>> Thanks,
>    >>>>
>    >>>> Dan
>    >>>>
>    >>>>
>    >>>>> version
>    >>>> _
>    >>>> platform i386-pc-linux-gnu
>    >>>> arch     i386
>    >>>> os       linux-gnu
>    >>>> system   i386, linux-gnu
>    >>>> status
>    >>>> major    2
>    >>>> minor    2.0
>    >>>> year     2005
>    >>>> month    10
>    >>>> day      06
>    >>>> svn rev  35749
>    >>>> language R
>    >>>>
>    >>>> ----------
>    >>>> Dan Davison
>    >>>> Committee on Evolutionary Biology
>    >>>> University of Chicago, U.S.A.
>    >>>>
>    >>>> ______________________________________________
>    >>>> R-devel at r-project.org mailing list
>    >>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>    >>>>
>    >>>
>    >>> --
>    >>> Roger D. Peng  |  http://www.biostat.jhsph.edu/~rpeng/
>    >>>
>    >>
>    >> ______________________________________________
>    >> R-devel at r-project.org mailing list
>    >> https://stat.ethz.ch/mailman/listinfo/r-devel
>    >>
>    >>
>
>    BDR> --
>    BDR> Brian D. Ripley,                  ripley at stats.ox.ac.uk
>    BDR> Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
>    BDR> University of Oxford,             Tel:  +44 1865 272861 (self)
>    BDR> 1 South Parks Road,                     +44 1865 272866 (PA)
>    BDR> Oxford OX1 3TG, UK                Fax:  +44 1865 272595
>
>    BDR> ______________________________________________
>    BDR> R-devel at r-project.org mailing list
>    BDR> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>

-- 
Brian D. Ripley,                  ripley at stats.ox.ac.uk
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595



[Rd] stopifnot() suggestion
Prof Brian Ripley