Henrik Bengtsson
2019-Jun-25 05:16 UTC
[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
**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
peter dalgaard
2019-Jun-25 07:44 UTC
[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
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
2019-Jun-25 09:48 UTC
[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
Ah, not quite: The logic is that if something has omittedSig and "missing" is not the signature, those signatures get _set_ to missing. There's just a bit of tap-dancing around to find exactly which they are so that there can be a message about changing them. -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
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
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