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

[Hervé Pagès]

Hi,

On 05/15/2017 10:41 AM, luke-tierney at uiowa.edu wrote:
> 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)

My main use case for using stopifnot() is argument checking. In that
context, I like the conciseness of

   stopifnot(
     A,
     B,
     ...
   )

I think it's a common use case (and a pretty natural thing to do) to
order/organize the expressions in a way such that it only makes sense
to continue evaluating if all was OK so far e.g.

   stopifnot(
     is.numeric(x),
     length(x) == 1,
     is.na(x)
   )

At least that's how things are organized in the stopifnot() calls that
accumulated in my code over the years. That's because I was convinced
that evaluation would stop at the first non-true expression (as
suggested by the man page). Until recently when I got a warning issued
by an expression located *after* the first non-true expression. This
was pretty unexpected/confusing!

If I can't rely on this "sequencing" feature, I guess I can always
do

   stopifnot(A)
   stopifnot(B)
   ...

but I loose the conciseness of calling stopifnot() only once.
I could also use

   stopifnot(A && B && ...)

but then I loose the conciseness of the error message i.e. it's going
to be something like

   Error: A && B && ... is not TRUE

which can be pretty long/noisy compared to the message that reports
only the 1st error.

Conciseness/readability of the single call to stopifnot() and
conciseness of the error message are the features that made me
adopt stopifnot() in the 1st place. If stopifnot() cannot be revisited
to do "sequencing" then that means I will need to revisit all my calls
to stopifnot().

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

Why not. Still better than the current situation. But only if that
semantic seems more useful to people. Would be sad if usefulness
of one semantic or the other was decided based on trickiness of
implementation.

Thanks,
H.

>
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__svn.r-2Dproject.org_R_trunk_src_library_base_R_stop.R&d=DwIFAw&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=t9fJDOl9YG2zB-GF0wQXrXJTsW2jxTxMHE-qZfLGzHU&s=KGsvpXrXpHCFTdbLM9ci3sBNO9C3ocsgEqHMvZKvV9I&e=
>>    >> or its corresponding github mirror
>>    >>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_wch_r-2Dsource_blob_trunk_src_library_base_R_stop.R&d=DwIFAw&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=t9fJDOl9YG2zB-GF0wQXrXJTsW2jxTxMHE-qZfLGzHU&s=7Z5bPVWdGPpY2KLnXQP6c-_8s86CpKe0ZYkCfqjfxY0&e=
>>    >> ]
>>    >>
>>    >> > 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
>>
>>
>

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