[R] [External] Re: A stopifnot() nastiness, even if not a bug
iuke-tier@ey m@iii@g oii uiow@@edu
iuke-tier@ey m@iii@g oii uiow@@edu
Sat May 9 20:13:17 CEST 2020
Since you asked ...
There are two different use cases for stopifnot: in tests and
in implementation code.
stopifnot seems to be optimized for the test code case where speed and
clarity may not be that important.
For expresing assertions in implemantation code clarity and simplicity
are important. What I would want is something equivalent to
stpifnt0 <- function(e)
if (! isTRUE(e))
stop(deparse(exprs[i]), "is not true")
(maybe with some additional gettetxt/deparse/call fiddling magic, but
only _after_ the fail, not on the hot path for a passed test.)
I would not want to have to worry about fine print about whether there
is an implicit 'all' for an expression returning a vector, or whether
that is really an 'all' or a variant that treate the empty set
differently.
I might consider a multiargument verison along the lines of
stpfnt <- function(...) {
exprs <- as.list(substitute(list(...)))[-1]
for (i in seq_along(exprs))
if (! isTRUE(...elt(i)))
stop(deparse(exprs[i]), "is not true")
}
But I think in most cases I rather write two separate assertions.
The advantage of stpfnt0 is code is that it is more concise than
writing out the test and the code to produce an error message,
especially a nice one. But it this comes at a big performance cost
then using it would be harder to jsutify.
With simple semantics like the stpfnt0 definition here the compiler
could be taught to optimize it's use to cost little more than the test
in the passing case. That gets harder the more complexity is added to
stopifnot.
We could add a simpler stopifnot0, but I'm not sure I want to go
there.
Best,
luke
On Sat, 9 May 2020, Martin Maechler wrote:
>>>>>> Martin Maechler
>>>>>> on Mon, 13 Apr 2020 22:30:35 +0200 writes:
>
>>>>>> William Dunlap
>>>>>> on Mon, 13 Apr 2020 09:57:11 -0700 writes:
>
> >> You can avoid the problem in Martin's example by only giving scalars to
> >> stopifnot(). E.g., using stopifnot(all(x>0)) or stopifnot(length(x)==1,
> x> 0) instead of stopifnot(x>0). I think having stopifnot call
> >> all(predicate) if length(predicate)!=1 was probably a mistake.
>
>> well, maybe.
>
>> As I brought up the 0-length example: One could think of making
>> an exception for logical(0) and treat that as non-TRUE.
>
>> (for R-devel only, [......])
>
>> Martin
>
> I have been a bit sad that nobody (not even Hervé) reacted to my
> proposal, 4 weeks ago.
>
> As I agree that it is safer for stopifnot() to be less lenient
> here, and not allow the usual behavior of logical(0) to be
> treated as TRUE, namely as in all(logical(0)) |--> TRUE ,
> I had actually implemented the above proposal in my own version of R-devel,
> (but not committed!), nicely introducing a new optional argument
> 'allow.logical0' where
>
> - allow.logical0 = FALSE is the new default
>
> - allow.logical0 = TRUE is back compatible
>
> What I found is that this (not back compatible) change lead to a
> few test breakages also in R & recommended packages, and IIRC in
> a few of my own packages. Still probably only in about 1 in 1000
> of the stopifnot cases, but in practically all cases, the breakage was
> "wrong" in the sense that {conceptual example}
>
> stopifnot( f1(x) == f2(x) )
>
> should test (almost, say apart from names(.)) identical behavior
> of f1() and f2() and that would naturally also extend to the
> case of 0-extent 'x'.
>
> So I had to change the above (half a dozen, say) cases to
>
> stopifnot( f1(x) == f2(x) , allow.logical0 = TRUE)
>
> to keep the test working as it was intended to.
> The nice thing about the change is that it is also working in
> current versions of R where allow.logical is not a special
> argument and just treated as part of '...' and is it TRUE, does
> not change the semantic of stopifnot() in current (possibly,
> then, "previous") versions of R.
>
> Overall I think it may be a good idea to consider this
> not-back-compatible change to stopifnot()
> (if only to get Hervé into continue using it ! ;-) ;-))
>
> BUT I assume quite a few other people may have to get used to
> see the following error in their stopifnot() code and will have
> to add occasional 'allow.logical0 = TRUE' to those cases the
> old behavior was really the intended one.
>
> (I will have to finally get Matrix 1.3-0 released to CRAN
> before committing the change to R-devel, and I may also ask
> help of someone to check all CRAN/Bioc against that change so I
> can alert package authors who need to adapt).
>
> Please let us know your thoughts on this.
>
> Martin
>
>
>
> >> On Mon, Apr 13, 2020 at 9:28 AM Hervé Pagès <hpages using fredhutch.org> wrote:
>
> >>>
> >>>
> >>> On 4/13/20 05:30, Martin Maechler wrote:
> >>> >>>>>> peter dalgaard
> >>> >>>>>> on Mon, 13 Apr 2020 12:00:38 +0200 writes:
> >>> >
> >>> > > Inline...
> >>> > >> On 13 Apr 2020, at 11:15 , Martin Maechler <
> >>> maechler using stat.math.ethz.ch> wrote:
> >>> > >>
> >>> > >>>>>>> Bert Gunter
> >>> > >>>>>>> on Sun, 12 Apr 2020 16:30:09 -0700 writes:
> >>> > >>
> >>> > >>> Don't know if this has come up before, but ...
> >>> > >>>> x <- c(0,0)
> >>> > >>>> length(x)
> >>> > >>> [1] 2
> >>> > >>> ## but
> >>> > >>>> stopifnot(length(x))
> >>> > >>> Error: length(x) is not TRUE
> >>> > >>> Called from: top level
> >>> > >>> ## but
> >>> > >>>> stopifnot(length(x) > 0) ## not an error; nor is
> >>> > >>>> stopifnot(as.logical(length(x)))
> >>> > >>> ## Ouch!
> >>> > >>
> >>> > >>> Maybe the man page should say something about not assuming
> >>> automatic
> >>> > >>> coercion to logical, which is the usual expectation. Or fix
> >>> this.
> >>> > >>
> >>> > >>> Bert Gunter
> >>> > >>
> >>> > >> Well, what about the top most paragraph of the help page is not
> >>> clear here ?
> >>> > >>
> >>> > >>> Description:
> >>> > >>
> >>> > >>> If any of the expressions (in '...' or 'exprs') are not 'all'
> >>> > >>> 'TRUE', 'stop' is called, producing an error message indicating
> >>> > >>> the _first_ expression which was not ('all') true.
> >>> > >>
> >>> >
> >>> > > This, however, is somewhat less clear:
> >>> >
> >>> > > ..., exprs: any number of (typically but not necessarily
> >>> ‘logical’) R
> >>> > > expressions, which should each evaluate to (a logical vector
> >>> > > of all) ‘TRUE’. Use _either_ ‘...’ _or_ ‘exprs’, the latter
> >>> >
> >>> > > What does it mean, "typically but not necessarily ‘logical’"?
> >>> >
> >>> > That's a good question: The '(....)' must have been put there a while
> >>> ago.
> >>> > I agree that it's not at all helpful. Strictly, we are really
> >>> > dealing with unevaluated expressions anyway ("promises"), but
> >>> > definitely all of them must evaluate to logical (vector or
> >>> > array..) of all TRUE values. In the very beginning of
> >>> > stopifnot(), I had thought that it should also work in other
> >>> > cases, e.g., for Matrix(TRUE, 4,5) {from the Matrix package} etc,
> >>> > but several use cases had convinced us / me that stopifnot
> >>> > should be stricter...
> >>> >
> >>> > > The code actually tests explicitly with is.logical, as far as I
> >>> can tell.
> >>> >
> >>> > > This creates a discrepancy between if(!...)stop(...) and
> >>> stopifnot(),
> >>> >
> >>> > yes indeed, on purpose now, for a very long time ...
> >>> >
> >>> > There's another discrepancy, more dangerous I think,
> >>> > as shown in the following
> >>> > {Note this discrepancy has been noted for a long time .. also on
> >>> > this R-devel list} :
> >>> >
> >>> > m <- matrix(1:12, 3,4)
> >>> > i <- (1:4) %% 2 == 1 & (0:3) %% 5 == 0
> >>> >
> >>> > stopifnot(dim(m[,i]) == c(3,1)) # seems fine
> >>> >
> >>> > if(dim(m[,i]) != c(3,1)) stop("wrong dim") # gives an error (but not
> >>> ..)
> >>>
> >>> mmh... that is not good. I was under the impression that we could at
> >>> least expect 'stopifnot(x)' to be equivalent to 'if (!isTRUE(x))
> >>> stop(...)'. I'll have to revisit my use of stopifnot() in many many
> >>> places... again :-/ Or may be just stop using it and use 'if
> >>> (!isTRUE(...))' instead.
> >>>
> >>> H.
> >>>
> >>> >
> >>> >
> >>> > Martin
> >>> >
> >>> > >> as in
> >>> > >> f <- function (x) if (!x) stop(paste(deparse(substitute(x)), "is
> >>> not TRUE"))
> >>> > >> f(0)
> >>> > > Error in f(0) : 0 is not TRUE
> >>> > >> f(1)
> >>> > >> stopifnot(0)
> >>> > > Error: 0 is not TRUE
> >>> > >> stopifnot(1)
> >>> > > Error: 1 is not TRUE
> >>> >
> >>> > > -pd
> >>> >
> >>> >
> >>> > >> If useR's expectations alone would guide the behavior of a
> >>> > >> computer language, the language would have to behave
> >>> > >> "personalized" and give different results depending on the user,
> >>> > >> which may be desirable in medicine or psychotherapy but not with
> >>> R.
>
> >>> > >> Martin
>
> ______________________________________________
> R-help using r-project.org mailing list -- To UNSUBSCRIBE and more, see
> https://stat.ethz.ch/mailman/listinfo/r-help
> PLEASE do read the posting guide http://www.R-project.org/posting-guide.html
> and provide commented, minimal, self-contained, reproducible code.
>
--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa Phone: 319-335-3386
Department of Statistics and Fax: 319-335-3017
Actuarial Science
241 Schaeffer Hall email: luke-tierney using uiowa.edu
Iowa City, IA 52242 WWW: http://www.stat.uiowa.edu
More information about the R-help
mailing list