>>>>> <luke-tier...@uiowa.edu> >>>>> on Tue, 16 May 2017 09:49:56 -0500 writes:
> On Tue, 16 May 2017, Martin Maechler wrote: >>>>>>> Hervé Pagès <hpa...@fredhutch.org> >>>>>>> on Mon, 15 May 2017 16:54:46 -0700 writes: >> >> > Hi, >> > On 05/15/2017 10:41 AM, luke-tier...@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 <so...@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 <so...@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-tier...@uiowa.edu > Iowa City, IA 52242 WWW: http://www.stat.uiowa.edu ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel