[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

peter dalgaard pd@|gd @end|ng |rom gm@||@com
Tue Jun 25 09:44:21 CEST 2019 

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 using 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 using 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 using 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 using 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 using 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 using cbs.dk  Priv: PDalgd using gmail.com
>> 
>> ______________________________________________
>> R-devel using r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
> 
> ______________________________________________
> R-devel using 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 using cbs.dk  Priv: PDalgd using gmail.com

More information about the R-devel mailing list