[Rd] stopifnot() does not stop at first non-TRUE argument
Martin Maechler
maechler at stat.math.ethz.ch
Tue May 16 19:10:30 CEST 2017
>>>>> <luke-tierney at uiowa.edu>
>>>>> on Tue, 16 May 2017 09:49:56 -0500 writes:
> On Tue, 16 May 2017, Martin Maechler wrote:
>>>>>>> Hervé Pagès <hpages at fredhutch.org>
>>>>>>> on Mon, 15 May 2017 16:54:46 -0700 writes:
>>
>> > 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)
>> > )
>>
>> I agree. And that's how I have used stopifnot() in many cases
>> myself, sometimes even more "extremely" than the above example,
>> using assertions that only make sense if previous assertions
>> were fulfilled, such as
>>
>> stopifnot(is.numeric(n), length(n) == 1, n == round(n), n >= 0)
>>
>> or in the Matrix package, first checking some class properties
>> and then things that only make sense for objects with those properties.
>>
>>
>> > 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.
>>
>> Yes, and that had been my design goal when I created it.
>>
>> I do tend agree with Hervé and Serguei here.
>>
>> > 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.
>>
>> Interesting... but really differing from the current documentation,
>>
>> > 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.
>>
>> Well, the trickiness should definitely play a role.
>> Apart from functionality and semantics, long term maintenance
>> and code readibility, even elegance have shown to be very
>> important aspects of good code in ca 30 years of S and R programming.
>>
>> OTOH, as mentioned above, the creation of good error messages
>> has been an important design goal of stopifnot() and hence I'm
>> willing to accept the extra complexity of "patching up" the call
>> used in the error / warning messages.
>>
>> Also, as a change to what I posted yesterday, I now plan to follow
>> Peter Dalgaard's suggestion of using
>> eval( ..<n> )
>> instead of eval(cl[[i]], envir = <parent.frame(.)>)
>> as there may be cases where the former behaves better in lazy
>> evaluation situations.
>> (Other opinions on that ?)
> If you go this route it would be useful to step back and think about
> whether there might be some useful primitives to add to make this
> easier, such as
> - provide a dotsLength function for computing the number arguments
> captured in a ... argument
actually my current version did not use that anymore
(and here we could use nargs() )
> - providing a dotsElt function for extracting the i-the element
> instead of going through the eval(sprintf("..%d", i)) construct.
I've started to do that, ...
> - maybe something for extracting the expression for the i-th argument.
well, yes, but that seems quite readable here, we use the whole
call anyway and loop, looking at each sequentially --- that part has not
been new!
> The might be more generally useful and make the code more readable and
> maintainable.
> Best,
> luke
>>
>> Martin
>>
>> > 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
>>
> --
> 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
More information about the R-devel
mailing list