Argh! Yes you are right, the "fix" doesn't. And I fell into the same "hey it's a vector so && has to be wrong"-trap. So this has to be reverted to something that has at least failed unconspicuously for a decade.... Will do. Thanks to Martin for remaining suspicious!
[This code was originally from 2009, by John Chambers. It is not too likely that he'll remember what the intention actually was... The logic does escape me: An omitted signature is only an omitted signature if the signature of the omitted signature is not "missing"? In that case, I think omittedSig[omittedSig] <- (signature[omittedSig] != "missing") might work (omittedSig[omittedSig] == TRUE, so we don't need to &. That is, unless there are NA issues.), but I am not at all sure that is what is wanted! ] -pd > On 25 Jun 2019, at 07:16 , Henrik Bengtsson <henrik.bengts...@gmail.com> > wrote: > > **Maybe this bug needs to be understood further before applying the > patch because patch is most likely also wrong** > > Because, from just looking at the expressions, I think neither the R > 3.6.0 version: > > omittedSig <- omittedSig && (signature[omittedSig] != "missing") > > nor the patched version (I proposed): > > omittedSig <- omittedSig & (signature[omittedSig] != "missing") > > can be correct. For a starter, 'omittedSig' is a logical vector. We > see that 'omittedSig' is used to subset 'signature'. In other words, > the length of 'signature[omittedSig]' and hence the length of > '(signature[omittedSig] != "missing")' will equal sum(omittedSig), > i.e. the length will be in {0,1,...,length(omittedSig)}. > > The R 3.6.0 version with '&&' is not correct because '&&' requires > length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely > to be the original intention. > > The patched version with '&' is most likely not correct either because > 'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the > auto-expansion of either vector. So, for the '&' version to be > correct, it basically requires that length(omittedSig) = length(LHS) = > length(RHS) = sum(omittedSig), which also sounds unlikely to be the > original intention. > > Disclaimer: Please note that I have not at all studied the rest of the > function, so the above is just based on that single line plus > debugging that 'omittedSig' is a logical vector. > > Martin, I don't have the time to dive into this further. Though I did > try to see if it happened when one of oligo's dependencies were > loaded, but that was not the case. It kicks in when oligo is loaded. > FYI, I can also replicate your non-replicatation on another R 3.6.0 > version. I'll see if I can narrow down what's different, e.g. > comparing sessionInfo():s, etc. However, I want to reply with > findings above asap due to the R 3.6.1 release and you might roll back > the patch (since it might introduce other bugs as Peter mentioned). > > /Henrik > > > On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler > <maech...@stat.math.ethz.ch> wrote: >> >>>>>>> Henrik Bengtsson via R-core >>>>>>> on Sun, 23 Jun 2019 11:29:58 -0700 writes: >> >>> Thank you. >>> To correct myself, I can indeed reproduce this with R --vanilla too. >>> A reproducible example is: >> >>> $ R --vanilla >>> R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree" >>> ... >>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true") >>>> loadNamespace("oligo") >>> Error in omittedSig && (signature[omittedSig] != "missing") : >>> 'length(x) = 4 > 1' in coercion to 'logical(1)' >>> Error: unable to load R code in package ‘oligo’ >> >>> /Henrik >> >> Thank you Henrik, for the report, etc, but >> hmm... after loading the oligo package, almost 40 (non >> base+Recommended) packages have been loaded as well, which hence >> need to have been installed before, too .. >> which is not quite a "vanilla repr.ex." in my view >> >> Worse, I cannot reproduce : >> >>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true") >>> Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_") >> [1] "true" >>> debugonce(conformMethod) >>> loadNamespace("oligo") >> <environment: namespace:oligo> >> Warning messages: >> 1: multiple methods tables found for ‘rowSums’ >> 2: multiple methods tables found for ‘colSums’ >> 3: multiple methods tables found for ‘rowMeans’ >> 4: multiple methods tables found for ‘colMeans’ >>> sessionInfo() >> R Under development (unstable) (2019-06-20 r76729) >> >> (similarly with other versions of R >= 3.6.0). >> >> So, even though R core has fixed this now in the sources, it >> would be nice to have an "as simple as possible" repr.ex. for this. >> >> Martin >> >> >> >>> On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <pda...@gmail.com> wrote: >>>> >>>> This looks obvious enough, so I just committed your fix to R-devel and >>>> R-patched. >>>> >>>> I'm at the wrong machine for thorough testing, but at least it seems to >>>> build OK. However, I sense some risk that this could uncover sleeping bugs >>>> elsewhere, so watch out. >>>> >>>> -pd >>>> >>>>> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <henrik.bengts...@gmail.com> >>>>> wrote: >>>>> >>>>> DISCLAIMER: I can not get this error with R --vanilla, so it only >>>>> occurs when some other package is also loaded. I don't have time to >>>>> find to narrow that down for a reproducible example, but I believe the >>>>> following error in R 3.6.0: >>>>> >>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true") >>>>>> library(oligo) >>>>> Error in omittedSig && (signature[omittedSig] != "missing") : >>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)' >>>>> Error: unable to load R code in package 'oligo' >>>>> >>>>> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the >>>>> 'methods' package. Here's the patch: >>>>> >>>>> $ svn diff src/library/methods/R/RMethodUtils.R & >>>>> [1] 1062 >>>>> Index: src/library/methods/R/RMethodUtils.R >>>>> =================================================================== >>>>> --- src/library/methods/R/RMethodUtils.R (revision 76731) >>>>> +++ src/library/methods/R/RMethodUtils.R (working copy) >>>>> @@ -343,7 +343,7 @@ >>>>> call. = TRUE, domain = NA) >>>>> } >>>>> else if(!all(signature[omittedSig] == "missing")) { >>>>> - omittedSig <- omittedSig && (signature[omittedSig] != "missing") >>>>> + omittedSig <- omittedSig & (signature[omittedSig] != "missing") >>>>> .message("Note: ", .renderSignature(f, sig0), >>>>> gettextf("expanding the signature to include omitted >>>>> arguments in definition: %s", >>>>> paste(sigNames[omittedSig], "= >>>>> \"missing\"",collapse = ", "))) >>>>> [1]+ Done svn diff >>>>> src/library/methods/R/RMethodUtils.R >>>>> >>>>> Maybe still in time for R 3.6.1? >>>>> >>>>> /Henrik >>>>> >>>>> ______________________________________________ >>>>> R-devel@r-project.org mailing list >>>>> https://stat.ethz.ch/mailman/listinfo/r-devel >>>> >>>> -- >>>> Peter Dalgaard, Professor, >>>> Center for Statistics, Copenhagen Business School >>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark >>>> Phone: (+45)38153501 >>>> Office: A 4.23 >>>> Email: pd....@cbs.dk Priv: pda...@gmail.com >> >> ______________________________________________ >> 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 -- Peter Dalgaard, Professor, Center for Statistics, Copenhagen Business School Solbjerg Plads 3, 2000 Frederiksberg, Denmark Phone: (+45)38153501 Office: A 4.23 Email: pd....@cbs.dk Priv: pda...@gmail.com ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel