thr3ads.net - R devel - [Rd] stopifnot() does not stop at first non-TRUE argument [May 2017]

If this information is useful, please help other people find it:
Share via:

Serguei Sokol

2017-May-15 14:32 UTC

[Rd] stopifnot() does not stop at first non-TRUE argument

Le 15/05/2017 ? 15:37, Martin Maechler a ?crit :>>>>>> Serguei Sokol <sokol at insa-toulouse.fr>
>>>>>>      on Mon, 15 May 2017 13:14:34 +0200 writes:
>      > I see in the archives that the attachment cannot pass.
>      > So, here is the code:
>
>      [....... MM: I needed to reformat etc to match closely to
>       the current source code which is in
>       	 https://svn.r-project.org/R/trunk/src/library/base/R/stop.R
>       or its corresponding github mirror
>         
https://github.com/wch/r-source/blob/trunk/src/library/base/R/stop.R
>      ]
>
>      > Best,
>      > Serguei.
>
> Yes, something like that seems even simpler than Peter's
> suggestion...
>
> It currently breaks 'make check' in the R sources,
> specifically in tests/reg-tests-2.R (lines 6574 ff),
> the new code now gives
>
>    > ## error messages from (C-level) evalList
>    > tst <- function(y) { stopifnot(is.numeric(y)); y+ 1 }
>    > try(tst())
>    Error in eval(cl.i, pfr) : argument "y" is missing, with no
default
>
> whereas previously it gave
>
>    Error in stopifnot(is.numeric(y)) :
>       argument "y" is missing, with no default
>
>
> But I think that change (of call stack in such an error case) is
> unavoidable and not a big problem.It can be avoided but at price of customizing error() and warning() calls with
something like:
wrn <- function(w) {w$call <- cl.i; warning(w)}
err <- function(e) {e$call <- cl.i; stop(e)}
...
tryCatch(r <- eval(cl.i, pfr), warning=wrn, error=err)

Serguei.
>
> --
>
> I'm still curious about Herv?'s idea on using  switch()  for the
> issue.
>
> Martin
>
>
>      > Le 15/05/2017 ? 12:48, Serguei Sokol a ?crit :
>      >> Hello,
>      >>
>      >> I am a new on this list, so I introduce myself very briefly:
>      >> my background is applied mathematics, more precisely
scientific calculus
>      >> applied for modeling metabolic systems, I am
author/maintainer of
>      >> few packages (Deriv, rmumps, arrApply).
>      >>
>      >> Now, on the subject of this discussion, I must say that I
don't really understand
>      >> Peter's argument:
>      >>
>      >> >>> To do it differently, you would have to do
something like
>      >> >>>
>      >> >>> dots <- match.call(expand.dots=FALSE)$...
>      >> >>>
>      >> >>> and then explicitly evaluate each argument in
turn in the caller
>      >> >>> frame. This amount of nonstandard evaluation
sounds like it would
>      >> >>> incur a performance penalty, which could be
undesirable.
>      >> The first line of the current stopifnot()
>      >> n <- length(ll <- list(...))
>      >> already evaluates _all_ of the arguments
>      >> in the caller frame. So to do the same only
>      >> on a part of them (till the first FALSE or NA occurs)
>      >> cannot be more penalizing than the current version, right?
>      >>
>      >> I attach here a slightly modified version called
stopifnot_new()
>      >> which works in accordance with the man page and
>      >> where there are only two additional calls: parent.frame() and
eval().
>      >> I don't think it can be considered as real performance
penalty
>      >> as the same or bigger amount of (implicit) evaluations was
>      >> already done in the current version:
>      >>
>      >>> source("stopifnot_new.R")
>      >>> stopifnot_new(3 == 5, as.integer(2^32), a <- 12)
>      >> Error: 3 == 5 is not TRUE
>      >>> a
>      >> Error: object 'a' not found
>      >>
>      >> Best,
>      >> Serguei.
>      >>
>      >>
>      >> Le 15/05/2017 ? 10:39, Martin Maechler a ?crit :
>      >>>>>>>> Herv? Pag?s <hpages at
fredhutch.org>
>      >>>>>>>> on Wed, 3 May 2017 12:08:26 -0700
writes:
>      >>> > On 05/03/2017 12:04 PM, Herv? Pag?s wrote:
>      >>> >> Not sure why the performance penalty of
nonstandard evaluation would
>      >>> >> be more of a concern here than for something
like switch().
>      >>>
>      >>> > which is actually a primitive. So it seems that
there is at least
>      >>> > another way to go than 'dots <-
match.call(expand.dots=FALSE)$...'
>      >>>
>      >>> > Thanks, H.
>      >>>
>      >>> >>
>      >>> >> If that can't/won't be fixed, what about
fixing the man page so it's
>      >>> >> in sync with the current behavior?
>      >>> >>
>      >>> >> Thanks, H.
>      >>>
>      >>> Being back from vacations,...
>      >>> I agree that something should be done here, if not to the
code than at
>      >>> least to the man page.
>      >>>
>      >>> For now, I'd like to look a bit longer into a
possible change to the function.
>      >>> Peter mentioned a NSE way to fix the problem and you
mentioned switch().
>      >>>
>      >>> Originally, stopifnot() was only a few lines of code and
meant to be
>      >>> "self-explaining" by just reading its
definition, and I really would like
>      >>> to not walk too much away from that original idea.
>      >>> How did you (Herve) think to use  switch()  here?
>      >>>
>      >>>
>      >>>
>      >>> >> On 05/03/2017 02:26 AM, peter dalgaard wrote:
>      >>> >>> The first line of stopifnot is
>      >>> >>>
>      >>> >>> n <- length(ll <- list(...))
>      >>> >>>
>      >>> >>> which takes ALL arguments and forms a list
of them. This implies
>      >>> >>> evaluation, so explains the effect that you
see.
>      >>> >>>
>      >>> >>> To do it differently, you would have to do
something like
>      >>> >>>
>      >>> >>> dots <- match.call(expand.dots=FALSE)$...
>      >>> >>>
>      >>> >>> and then explicitly evaluate each argument
in turn in the caller
>      >>> >>> frame. This amount of nonstandard evaluation
sounds like it would
>      >>> >>> incur a performance penalty, which could be
undesirable.
>      >>> >>>
>      >>> >>> If you want to enforce the order of
evaluation, there is always
>      >>> >>>
>      >>> >>> stopifnot(A) stopifnot(B)
>      >>> >>>
>      >>> >>> -pd
>      >>> >>>
>      >>> >>>> On 3 May 2017, at 02:50 , Herv? Pag?s
<hpages at fredhutch.org>
>      >>> >>>> wrote:
>      >>> >>>>
>      >>> >>>> Hi,
>      >>> >>>>
>      >>> >>>> It's surprising that stopifnot()
keeps evaluating its arguments
>      >>> >>>> after it reaches the first one that is
not TRUE:
>      >>> >>>>
>      >>> >>>> > stopifnot(3 == 5, as.integer(2^32),
a <- 12) Error: 3 == 5 is
>      >>> >>>> not TRUE In addition: Warning message:
In stopifnot(3 == 5,
>      >>> >>>> as.integer(2^32), a <- 12) : NAs
introduced by coercion to integer
>      >>> >>>> range > a [1] 12
>      >>> >>>>
>      >>> >>>> The details section in its man page
actually suggests that it
>      >>> >>>> should stop at the first non-TRUE
argument:
>      >>> >>>>
>      >>> >>>> ?stopifnot(A, B)? is conceptually
equivalent to
>      >>> >>>>
>      >>> >>>> { if(any(is.na(A)) || !all(A))
stop(...); if(any(is.na(B)) ||
>      >>> >>>> !all(B)) stop(...) }
>      >>> >>>>
>      >>> >>>> Best, H.
>      >>> >>>>
>      >>> >>>> --
>      >>> >>>> Herv? Pag?s
>      >>> >>>>
>      >>> >>>> Program in Computational Biology
Division of Public Health
>      >>> >>>> Sciences Fred Hutchinson Cancer Research
Center 1100 Fairview
>      >>> >>>> Ave. N, M1-B514 P.O. Box 19024 Seattle,
WA 98109-1024
>      >>> >>>>
>      >>> >>>> E-mail: hpages at fredhutch.org Phone:
(206) 667-5791 Fax: (206)
>      >>> >>>> 667-1319
>      >>> >>>>
>      >>> >>>>
______________________________________________
>      >>> >>>> R-devel at r-project.org mailing list
>      >>> >>>>
>      >>>
https://urldefense.proofpoint.com/v2/url?u=https-3A__stat.ethz.ch_mailman_listinfo_r-2Ddevel&d=DwIFaQ&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=JwgKhKD2k-9Kedeh6pqu-A8x6UEV0INrcxcSGVGo3Tg&s=f7IKJIhpRNJMC3rZAkuI6-MTdL3GAKSV2wK0boFN5HY&e>
>>> >>>>
>      >>> >>>
>      >>> >>
>      >>>
>      >>> > -- Herv? Pag?s
>      >>>
>      >>> > Program in Computational Biology Division of Public
Health Sciences
>      >>> > Fred Hutchinson Cancer Research Center 1100 Fairview
Ave. N,
>      >>> > M1-B514 P.O. Box 19024 Seattle, WA 98109-1024
>      >>>
>      >>> > E-mail: hpages at fredhutch.org Phone: (206)
667-5791 Fax: (206)
>      >>> > 667-1319
>      >>>
>      >>> > ______________________________________________
>      >>> > R-devel at r-project.org mailing list
>      >>> > https://stat.ethz.ch/mailman/listinfo/r-devel
>      >>>
>      >>> ______________________________________________
>      >>> R-devel at r-project.org mailing list
>      >>> https://stat.ethz.ch/mailman/listinfo/r-devel
>      >>
>
>      > ______________________________________________
>      > R-devel at r-project.org mailing list
>      > https://stat.ethz.ch/mailman/listinfo/r-devel
>

Martin Maechler

2017-May-15 15:44 UTC

head link

[Rd] stopifnot() does not stop at first non-TRUE argument

>>>>> Serguei Sokol <sokol at insa-toulouse.fr>
>>>>>     on Mon, 15 May 2017 16:32:20 +0200 writes:
    > Le 15/05/2017 ? 15:37, Martin Maechler a ?crit :
    >>>>>>> Serguei Sokol <sokol at insa-toulouse.fr>
    >>>>>>> on Mon, 15 May 2017 13:14:34 +0200 writes:
    >> > I see in the archives that the attachment cannot pass.
    >> > So, here is the code:
    >> 
    >> [....... MM: I needed to reformat etc to match closely to
    >> the current source code which is in
    >> https://svn.r-project.org/R/trunk/src/library/base/R/stop.R
    >> or its corresponding github mirror
    >>
https://github.com/wch/r-source/blob/trunk/src/library/base/R/stop.R
    >> ]
    >> 
    >> > Best,
    >> > Serguei.
    >> 
    >> Yes, something like that seems even simpler than Peter's
    >> suggestion...
    >> 
    >> It currently breaks 'make check' in the R sources,
    >> specifically in tests/reg-tests-2.R (lines 6574 ff),
    >> the new code now gives
    >> 
    >> > ## error messages from (C-level) evalList
    >> > tst <- function(y) { stopifnot(is.numeric(y)); y+ 1 }
    >> > try(tst())
    >> Error in eval(cl.i, pfr) : argument "y" is missing, with
no default
    >> 
    >> whereas previously it gave
    >> 
    >> Error in stopifnot(is.numeric(y)) :
    >> argument "y" is missing, with no default
    >> 
    >> 
    >> But I think that change (of call stack in such an error case) is
    >> unavoidable and not a big problem.

    > It can be avoided but at price of customizing error() and warning()
calls with something like:
    > wrn <- function(w) {w$call <- cl.i; warning(w)}
    > err <- function(e) {e$call <- cl.i; stop(e)}
    > ...
    > tryCatch(r <- eval(cl.i, pfr), warning=wrn, error=err)

    > Serguei.

Well, a good idea, but the 'warning' case is more complicated
(and the above incorrect): I do want the warning there, but
_not_ return the warning, but rather, the result of eval() :
So this needs even more sophistication, using  withCallingHandlers(.)
and maybe that really get's too sophisticated and no
more "readable" to 99.9% of the R users ... ?

I now do append my current version -- in case some may want to
comment or improve further.

Martin

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: stopifnot_new.R.~3~
URL:
<https://stat.ethz.ch/pipermail/r-devel/attachments/20170515/4d595547/attachment.ksh>

Serguei Sokol

2017-May-15 17:24 UTC

head link

[Rd] stopifnot() does not stop at first non-TRUE argument

Le 15/05/2017 ? 17:44, Martin Maechler a ?crit :
...> So this needs even more sophistication, using withCallingHandlers(.)
> and maybe that really get's too sophisticated and no
> more "readable" to 99.9% of the R users ... ?I'd say the current version is of minimal sophistication to reach
both the doc and test requirements.

Serguei.

luke-tierney at uiowa.edu

2017-May-15 17:41 UTC

head link

[Rd] stopifnot() does not stop at first non-TRUE argument

This is getting pretty convoluted.

The current behavior is consistent with the description at the top of
the help page -- it does not promise to stop evaluation once the first
non-TRUE is found.  That seems OK to me -- if you want sequencing you
can use

stopifnot(A)
stopifnot(B)

or

stopifnot(A && B)

I could see an argument for a change that in the multiple argumetn
case reports _all_ that fail; that would seem more useful to me than
twisting the code into knots.

Best,

luke

On Mon, 15 May 2017, Martin Maechler wrote:
>>>>>> Serguei Sokol <sokol at insa-toulouse.fr>
>>>>>>     on Mon, 15 May 2017 16:32:20 +0200 writes:
>
>    > Le 15/05/2017 ? 15:37, Martin Maechler a ?crit :
>    >>>>>>> Serguei Sokol <sokol at
insa-toulouse.fr>
>    >>>>>>> on Mon, 15 May 2017 13:14:34 +0200 writes:
>    >> > I see in the archives that the attachment cannot pass.
>    >> > So, here is the code:
>    >>
>    >> [....... MM: I needed to reformat etc to match closely to
>    >> the current source code which is in
>    >> https://svn.r-project.org/R/trunk/src/library/base/R/stop.R
>    >> or its corresponding github mirror
>    >>
https://github.com/wch/r-source/blob/trunk/src/library/base/R/stop.R
>    >> ]
>    >>
>    >> > Best,
>    >> > Serguei.
>    >>
>    >> Yes, something like that seems even simpler than Peter's
>    >> suggestion...
>    >>
>    >> It currently breaks 'make check' in the R sources,
>    >> specifically in tests/reg-tests-2.R (lines 6574 ff),
>    >> the new code now gives
>    >>
>    >> > ## error messages from (C-level) evalList
>    >> > tst <- function(y) { stopifnot(is.numeric(y)); y+ 1 }
>    >> > try(tst())
>    >> Error in eval(cl.i, pfr) : argument "y" is missing,
with no default
>    >>
>    >> whereas previously it gave
>    >>
>    >> Error in stopifnot(is.numeric(y)) :
>    >> argument "y" is missing, with no default
>    >>
>    >>
>    >> But I think that change (of call stack in such an error case)
is
>    >> unavoidable and not a big problem.
>
>    > It can be avoided but at price of customizing error() and warning()
calls with something like:
>    > wrn <- function(w) {w$call <- cl.i; warning(w)}
>    > err <- function(e) {e$call <- cl.i; stop(e)}
>    > ...
>    > tryCatch(r <- eval(cl.i, pfr), warning=wrn, error=err)
>
>    > Serguei.
>
> Well, a good idea, but the 'warning' case is more complicated
> (and the above incorrect): I do want the warning there, but
> _not_ return the warning, but rather, the result of eval() :
> So this needs even more sophistication, using  withCallingHandlers(.)
> and maybe that really get's too sophisticated and no
> more "readable" to 99.9% of the R users ... ?
>
> I now do append my current version -- in case some may want to
> comment or improve further.
>
> Martin
>
>
-- 
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 at uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

Reasonably Related Threads

Search for more possibly parallel threads

R devel - May 2017 - stopifnot() does not stop at first non-TRUE argument

[Rd] stopifnot() does not stop at first non-TRUE argument

[Rd] stopifnot() does not stop at first non-TRUE argument

[Rd] stopifnot() does not stop at first non-TRUE argument

[Rd] stopifnot() does not stop at first non-TRUE argument

Reasonably Related Threads