luke-tierney at uiowa.edu
2017-May-15 17:41 UTC
[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
Hervé Pagès
2017-May-15 23:54 UTC
[Rd] stopifnot() does not stop at first non-TRUE argument
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
Martin Maechler
2017-May-16 09:31 UTC
[Rd] stopifnot() does not stop at first non-TRUE argument
>>>>> 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 ?) 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
Serguei Sokol
2017-May-16 09:37 UTC
[Rd] stopifnot() does not stop at first non-TRUE argument
Le 15/05/2017 ? 19:41, luke-tierney at uiowa.edu a ?crit :> 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.Hm... we can read in the man page : ?stopifnot(A, B)? is conceptually equivalent to { if(any(is.na(A)) || !all(A)) stop(...); if(any(is.na(B)) || !all(B)) stop(...) } and this behavior does promise to stop at first non-TRUE value without evaluation of the rest of conditions. Sergue?.
luke-tierney at uiowa.edu
2017-May-16 14:40 UTC
[Rd] stopifnot() does not stop at first non-TRUE argument
On Tue, 16 May 2017, Serguei Sokol wrote:> Le 15/05/2017 ? 19:41, luke-tierney at uiowa.edu a ?crit : >> 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. > Hm... we can read in the man page : > ?stopifnot(A, B)? is conceptually equivalent to > > { if(any(is.na(A)) || !all(A)) stop(...); > if(any(is.na(B)) || !all(B)) stop(...) } > and this behavior does promise to stop at first non-TRUE value > without evaluation of the rest of conditions.Yes: that is why I explicitly referenced the description at the top of the page. Changing the 'conceptually equivalent' bit to reflect what is happening is easy. The changes being discussed, and their long term maintenance, ar not. Best, luke> > Sergue?. >-- 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
Apparently Analagous Threads
- stopifnot() does not stop at first non-TRUE argument
- stopifnot() does not stop at first non-TRUE argument
- stopifnot() does not stop at first non-TRUE argument
- stopifnot() does not stop at first non-TRUE argument
- stopifnot() does not stop at first non-TRUE argument