peter dalgaard
2019-Jun-27 14:23 UTC
[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
Henrik,
If a minimal reprex is hard to construct, could you perhaps instrument your
version of R to include a browser() call at the start of the
else if(!all(signature[omittedSig] == "missing")) {
branch, run the code that triggers the issue for you (and must hit that branch)
and tell us what the "signature" and "omittedSig" objects
look like at that point?
Bonus points for figuring out why it is needed to use numerical indexing in
omittedSig <- seq_along(omittedSig)[omittedSig]
signature[omittedSig] <- "missing" # logical index will
extend signature!
(I'm sure there is a valid reason, I just don't get it...)
-pd
> On 25 Jun 2019, at 09:44 , peter dalgaard <pdalgd at gmail.com>
wrote:
>
> 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.bengtsson at
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
>> <maechler at 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 <pdalgd at
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.bengtsson at 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 at 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.mes at cbs.dk Priv: PDalgd at gmail.com
>>>
>>> ______________________________________________
>>> R-devel at r-project.org mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>> ______________________________________________
>> R-devel at 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.mes at cbs.dk Priv: PDalgd at gmail.com
>
>
>
>
>
>
>
>
>
--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: pd.mes at cbs.dk Priv: PDalgd at gmail.com
Martin Maechler
2019-Jun-27 15:15 UTC
[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
>>>>> peter dalgaard >>>>> on Thu, 27 Jun 2019 16:23:14 +0200 writes:> Henrik, > If a minimal reprex is hard to construct, could you perhaps instrument your version of R to include a browser() call at the start of the > else if(!all(signature[omittedSig] == "missing")) { > branch, run the code that triggers the issue for you (and must hit that branch) and tell us what the "signature" and "omittedSig" objects look like at that point? > Bonus points for figuring out why it is needed to use numerical indexing in > omittedSig <- seq_along(omittedSig)[omittedSig] > signature[omittedSig] <- "missing" # logical index will extend signature! > (I'm sure there is a valid reason, I just don't get it...) > -pd I've also have mused over that question... and I had assumed some difference in the case the original omittedSig contains NAs ... but that's NOT true actually, see: > sign2 <- signatures <- LETTERS > omittedSig <- LETTERS < "K" > omittedSig[c(8,18)] <- NA # now have an omittedSig with {T, F, NA} > iSig <- seq_along(omittedSig)[omittedSig] > sign2[iSig] <- "missing" > signatures[omittedSig] <- "missing" > identical(sign2, signatures) [1] TRUE > so I still don't see the case where it makes a difference. Martin >> On 25 Jun 2019, at 09:44 , peter dalgaard <pdalgd at gmail.com> wrote: >> >> 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.bengtsson at 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 >>> <maechler at 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 <pdalgd at 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.bengtsson at 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 at 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.mes at cbs.dk Priv: PDalgd at gmail.com>>>> >>>> ______________________________________________ >>>> R-devel at r-project.org mailing list >>>> https://stat.ethz.ch/mailman/listinfo/r-devel >>> >>> ______________________________________________ >>> R-devel at 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.mes at cbs.dk Priv: PDalgd at gmail.com >> >> >> >> >> >> >> >> >> > -- > Peter Dalgaard, Professor, > Center for Statistics, Copenhagen Business School > Solbjerg Plads 3, 2000 Frederiksberg, Denmark > Phone: (+45)38153501 > Office: A 4.23 > Email: pd.mes at cbs.dk Priv: PDalgd at gmail.com
Henrik Bengtsson
2019-Jun-27 23:00 UTC
[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
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'
This is with:
sessionInfo()
R version 3.6.0 Patched (2019-06-23 r76734)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.2 LTS
Matrix products: default
BLAS: /home/hb/software/R-devel/R-3-6-branch/lib/R/lib/libRblas.so
LAPACK: /home/hb/software/R-devel/R-3-6-branch/lib/R/lib/libRlapack.so
locale:
[1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C
[3] LC_TIME=en_US.UTF-8 LC_COLLATE=en_US.UTF-8
[5] LC_MONETARY=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8
[7] LC_PAPER=en_US.UTF-8 LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C
attached base packages:
[1] stats graphics grDevices utils datasets methods base
loaded via a namespace (and not attached):
[1] Rcpp_1.0.1 affxparser_1.56.0
[3] XVector_0.24.0 splines_3.6.0
[5] GenomicRanges_1.36.0 BiocGenerics_0.30.0
[7] zlibbioc_1.30.0 IRanges_2.18.1
[9] bit_1.1-14 BiocParallel_1.18.0
[11] lattice_0.20-38 foreach_1.4.4
[13] blob_1.1.1 GenomeInfoDb_1.20.0
[15] tools_3.6.0 SummarizedExperiment_1.14.0
[17] parallel_3.6.0 grid_3.6.0
[19] Biobase_2.44.0 ff_2.2-14
[21] DBI_1.0.0 iterators_1.0.10
[23] oligoClasses_1.46.0 matrixStats_0.54.0
[25] digest_0.6.19 bit64_0.9-7
[27] preprocessCore_1.46.0 affyio_1.54.0
[29] Matrix_1.2-17 GenomeInfoDbData_1.2.1
[31] BiocManager_1.30.4 codetools_0.2-16
[33] S4Vectors_0.22.0 bitops_1.0-6
[35] RCurl_1.95-4.12 memoise_1.1.0
[37] RSQLite_2.1.1 DelayedArray_0.10.0
[39] compiler_3.6.0 Biostrings_2.52.0
[41] stats4_3.6.0
/Henrik
On Thu, Jun 27, 2019 at 8:16 AM Martin Maechler
<maechler at stat.math.ethz.ch> wrote:>
> >>>>> peter dalgaard
> >>>>> on Thu, 27 Jun 2019 16:23:14 +0200 writes:
>
> > Henrik,
> > If a minimal reprex is hard to construct, could you perhaps
instrument your version of R to include a browser() call at the start of the
>
> > else if(!all(signature[omittedSig] == "missing")) {
>
> > branch, run the code that triggers the issue for you (and must hit
that branch) and tell us what the "signature" and
"omittedSig" objects look like at that point?
>
> > Bonus points for figuring out why it is needed to use numerical
indexing in
>
> > omittedSig <- seq_along(omittedSig)[omittedSig]
> > signature[omittedSig] <- "missing" # logical index
will extend signature!
>
> > (I'm sure there is a valid reason, I just don't get it...)
>
> > -pd
>
> I've also have mused over that question...
> and I had assumed some difference in the case the original
> omittedSig contains NAs ... but that's NOT true actually, see:
>
> > sign2 <- signatures <- LETTERS
> > omittedSig <- LETTERS < "K"
> > omittedSig[c(8,18)] <- NA # now have an omittedSig with {T, F,
NA}
> > iSig <- seq_along(omittedSig)[omittedSig]
> > sign2[iSig] <- "missing"
> > signatures[omittedSig] <- "missing"
> > identical(sign2, signatures)
> [1] TRUE
> >
>
> so I still don't see the case where it makes a difference.
>
> Martin
>
> >> On 25 Jun 2019, at 09:44 , peter dalgaard <pdalgd at
gmail.com> wrote:
> >>
> >> 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.bengtsson at 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
> >>> <maechler at 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
<pdalgd at 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.bengtsson at 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 at 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.mes at cbs.dk Priv: PDalgd at gmail.com
> >>>>
> >>>> ______________________________________________
> >>>> R-devel at r-project.org mailing list
> >>>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >>>
> >>> ______________________________________________
> >>> R-devel at 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.mes at cbs.dk Priv: PDalgd at gmail.com
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
>
> > --
> > Peter Dalgaard, Professor,
> > Center for Statistics, Copenhagen Business School
> > Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> > Phone: (+45)38153501
> > Office: A 4.23
> > Email: pd.mes at cbs.dk Priv: PDalgd at gmail.com
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
Maybe Matching Threads
- methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
- methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
- methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
- methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
- methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error