I still observe this error and just want to ping this thread so we don't forget it. Should I add this to https://bugs.r-project.org/bugzilla/ so it's tracked there?
This thread in the archives: * https://stat.ethz.ch/pipermail/r-devel/2019-June/078049.html * https://stat.ethz.ch/pipermail/r-devel/2019-July/078115.html * https://stat.ethz.ch/pipermail/r-devel/2019-July/078126.html /Henrik On Sat, Jun 29, 2019 at 1:45 PM Martin Maechler <maech...@stat.math.ethz.ch> wrote: > > >>>>> Martin Maechler > >>>>> on Sat, 29 Jun 2019 12:05:49 +0200 writes: > > >>>>> Martin Maechler > >>>>> on Sat, 29 Jun 2019 10:33:10 +0200 writes: > > >>>>> peter dalgaard > >>>>> on Fri, 28 Jun 2019 16:20:03 +0200 writes: > > >>> > On 28 Jun 2019, at 16:03 , Martin Maechler > <maech...@stat.math.ethz.ch> wrote: > >>> > > >>> >>>>>> Henrik Bengtsson > >>> >>>>>> on Thu, 27 Jun 2019 16:00:39 -0700 writes: > >>> > > >>> >> Using: > >>> >> > >>> >> untrace(methods::conformMethod) > >>> >> at <- c(12,4,3,2) > >>> >> str(body(methods::conformMethod)[[at]]) > >>> >> ## language omittedSig <- omittedSig && (signature[omittedSig] != > "missing") > >>> >> cc <- 0L > >>> >> trace(methods::conformMethod, tracer = quote({ > >>> >> cc <<- cc + 1L > >>> >> print(cc) > >>> >> if (cc == 31) { ## manually identified > >>> >> untrace(methods::conformMethod) > >>> >> trace(methods::conformMethod, at = list(at), tracer = quote({ > >>> >> str(list(signature = signature, mnames = mnames, fnames = > fnames)) > >>> >> print(ls()) > >>> >> try(str(list(omittedSig = omittedSig, signature = signature))) > >>> >> })) > >>> >> } > >>> >> })) > >>> >> loadNamespace("oligo") > >>> >> > >>> >> gives: > >>> >> > >>> >> Untracing function "conformMethod" in package "methods" > >>> >> Tracing function "conformMethod" in package "methods" > >>> >> Tracing conformMethod(signature, mnames, fnames, f, fdef, > definition) > >>> >> step 12,4,3,2 > >>> >> List of 3 > >>> >> $ signature: Named chr [1:4] "TilingFeatureSet" "ANY" "ANY" "array" > >>> >> ..- attr(*, "names")= chr [1:4] "object" "subset" "target" "value" > >>> >> ..- attr(*, "package")= chr [1:4] "oligoClasses" "methods" > "methods" "methods" > >>> >> $ mnames : chr [1:2] "object" "value" > >>> >> $ fnames : chr [1:4] "object" "subset" "target" "value" > >>> >> [1] "f" "fdef" "fnames" "fsig" "imf" > >>> >> [6] "method" "mnames" "omitted" "omittedSig" "sig0" > >>> >> [11] "sigNames" "signature" > >>> >> List of 2 > >>> >> $ omittedSig: logi [1:4] FALSE TRUE TRUE FALSE > >>> >> $ signature : Named chr [1:4] "TilingFeatureSet" "ANY" "ANY" > "array" > >>> >> ..- attr(*, "names")= chr [1:4] "object" "subset" "target" "value" > >>> >> ..- attr(*, "package")= chr [1:4] "oligoClasses" "methods" > "methods" "methods" > >>> >> Error in omittedSig && (signature[omittedSig] != "missing") : > >>> >> 'length(x) = 4 > 1' in coercion to 'logical(1)' > >>> >> Error: unable to load R code in package 'oligo' > >>> >> > >>> > > >>> > Thank you, Henrik, nice piece of using trace() .. and the above > >>> > is useful for solving the issue -- I can work with that. > >>> > > >>> > I'm already pretty sure the wrong code starts with > >>> > > >>> > omittedSig <- sigNames %in% fnames[omitted] # .... > > >> my "pretty sure" statement above has proven to be wrong .. > > >>> > ------------- > >>> > > >>> > >>> I think the intention must have been that the two "ANY" signatures > should change to "missing". > > >> Definitely. > > >>> However, with the current logic that will not happen, because > >>> > >>> > c(F,T,T,F) && c(T,T) > >>> [1] FALSE > >>> > >>> Henrik's non-fix would have resulted in > >>> > >>> > c(F,T,T,F) & c(T,T) > >>> [1] FALSE TRUE TRUE FALSE > >>> > >>> which is actually right, but only coincidentally due to recycling of > c(T,T). Had it been c(F,T) then it would have been expanded to c(F,T,F,T) > which would be the opposite of what was wanted. > >>> > >>> Barring NA issues, I still think > >>> > >>> omittedSig[omittedSig] <- (signature[omittedSig] != "missing") > >>> > >>> should do the trick. > > >> yes, (most probably). I've found a version of that which should > >> be even easier to "read and understand", in svn commit 76753 : > > >> svn diff -c 76753 src/library/methods/R/RMethodUtils.R > > >> --- src/library/methods/R/RMethodUtils.R (Revision 76752) > >> +++ src/library/methods/R/RMethodUtils.R (Revision 76753) > >> @@ -342,8 +342,7 @@ > >> gettextf("formal arguments (%s) omitted in the method definition > cannot be in the signature", bad2), > >> call. = TRUE, domain = NA) > >> } > >> - else if(!all(signature[omittedSig] == "missing")) { > >> - omittedSig <- omittedSig && (signature[omittedSig] != > "missing") > >> + else if(any(omittedSig <- omittedSig & signature != "missing")) { > > > >> BTW: I've marked this --- and the runmed() seg.fault + na.action > >> change --- as something to be added to R 3.6.1 patched, as I > >> deemed I should obey the "code freeze" rule in both cases. > > >> Martin > > > Hmm... I think we got a 'Neverending Story' here -- because it > > seems, both Peter and I were wrong in thinking that it's a good > > idea to change "missing" to "ANY" here ... > > ((or if that *was* correct, that needs to entail more changes > > happening during setMethod(.) {conformMethod() is only called in > > one place in our code base, namely from setMethod()} : > > as a matter of fact, I've been brave for now, left the change to > conformMethod() and started to fix the constructed .local() > calls which are created in conformMethod()'s sibling, > which is rematchDefinition(). > > It seems that this builds e.g. Matrix {with its gazillion > setMethod()s} and that continues to run its own tests. OTOH, > Matrix may not trigger the situations that are dealt with here > at all, as the signature() are rarely longer than three, and at > some point in time I had made long passes through the package in > order to "minimize" the .local() calls. > > --> svn commit 76756 (in addition to 76753, mentioned earlier) > now has rematchDefinition() changes > > I would be positively surprised if (but can imagine that) this > had no affect on CRAN / Bioconductor packages. > > Still, these two changes seem to achieve what both the comments > and the documentation of conformMethod() and rematchDefinition() > suggest should happen. > > Of course, I'd really be happy if people with S4 packages would > check them with an R-devel version with svn rev >= 76756 > and report problems they see. > I do imagine effects, and would expect that bugs in current code > become visible where they had not done so previously. > > Martin > > > I've started to explore the effects of the change using and > > extending the tests/reg-tests-1d.R example that I had just committed. > > > And the result is *not good* : > > > As we said above, the new code does replace all "ANY" by "missing" in > the > > signature, unless the "ANY" are at the end of the signature. > > > However, later methods package code producing the .local(.) > > calls in the method definition (in cases where the signature of > > the generic and the method do not match exactly) --- possibly > > may have been tweaked later than the conformMethod() code --- > > and the .local() calls they now produce > > > - work as intended for R <= 3.6.0 (and R 3.6.1 RC) > > - fail to work for R-devel with the change : > > > See this (not a minimal rep.rex. but one building on Henrik's > > oligo case and what's newly in tests/reg-tests-1d.R ) : > > > > ----------------------------------------------------------------------------- > > > ## conformMethod() "&& logic" bug, by Henrik Bengtsson on R-devel > list, 2019-06-22 > > setClass("tilingFSet", slots = c(x = "numeric")) > > if(!is.null(getGeneric("oligoFn"))) removeGeneric("oligoFn") > > setGeneric("oligoFn", > > function(object, subset, target, value) { standardGeneric("oligoFn") }) > > Sys.unsetenv("_R_CHECK_LENGTH_1_LOGIC2_") ## << added here, to compare > with R 3.6.0, 3.5.3, .. > > setMethod("oligoFn", signature(object = "tilingFSet", value="array"), > ## Method _ 1 _ > > function(object, value) { list(object=object, value=value) }) > > setMethod("oligoFn", signature(object = "matrix", target="array"), > ## Method _ 2 _ > > function(object, target) list(object=object, target=target)) > > setMethod("oligoFn", signature(object = "matrix", subset="integer"), > ## Method _ 3 _ > > function(object, subset) list(object=object, subset=subset)) # > (*no* Note: ANY at end) > > setMethod("oligoFn", signature(object = "matrix"), > ## Method _ 4 _ > > function(object) list(object=object)) # > (*no* Note: ANY at end) > > showMethods("oligoFn") # F.Y.I.: in R 3.6.0 and earlier: contains > "ANY" everywhere > > > ##-- Now, the following *DOES* work fine in R <= 3.6.0 but *no longer* > in R-devel : > > str(r1 <- oligoFn(object=new("tilingFSet"), value=array(2))) > > str(r2 <- oligoFn(object=diag(2), target=array(42))) > > ## These 2 work fine in all versions of R: Here the "ANY" remain at the > end: > > str(r3 <- oligoFn(object=diag(2), subset=1:3)) > > str(r4 <- oligoFn(object=diag(2))) > > > > ----------------------------------------------------------------------------- > > The two errors in R-devel are actually quite user-confusing: > > >> r2 <- oligoFn(object=diag(2), target=array(42)) > > Error in .local(object, target) : > > argument "target" is missing, with no default > >> getMethod("oligoFn", signature(object="matrix", subset="missing", > target="array")) > > Method Definition: > > > function (object, subset, target, value) > > { > > .local <- function (object, subset, target, value) > > list(object = object, target = target) > > .local(object, target) > > } > > > Signatures: > > object subset target value > > target "matrix" "missing" "array" "ANY" > > defined "matrix" "missing" "array" "ANY" > >> > > > > My conclusion: There's something really wrong with what > > conformMethod() has been *intended* to achieve and what it > > "accidentally" did achieve in these cases: it never replaced > > "ANY" by "missing" in all these cases *AND* that is what it > > seems it should continue doing. > > > BTW: ?conformMethod goes to a page with quite a few things, > > relevantly containing > > > ‘conformMethod’: If the formal arguments, ‘mnames’, are not > > identical to the formal arguments to the function, ‘fnames’, > > ‘conformMethod’ determines whether the signature and the two > > sets of arguments conform, and returns the signature, > > possibly extended. The function name, ‘f’ is supplied for > > error messages. The generic function, ‘fdef’, supplies the > > generic signature for matching purposes. > > > The method assignment conforms if method and generic function > > have identical formal argument lists. It can also conform if > > the method omits some of the formal arguments of the function > > but: (1) the non-omitted arguments are a subset of the > > function arguments, appearing in the same order; (2) there > > are no arguments to the method that are not arguments to the > > function; and (3) the omitted formal arguments do not appear > > as explicit classes in the signature. A future extension > > hopes to test also that the omitted arguments are not assumed > > by being used as locally assigned names or function names in > > the body of the method. > > > --- > > It seems my commit to R-devel needs another change. > > This has to wait for hours currently, though. > > > Martin > > > ______________________________________________ > > R-devel@r-project.org mailing list > > https://stat.ethz.ch/mailman/listinfo/r-devel > > ______________________________________________ > R-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
