From pd@|gd @end|ng |rom gm@||@com  Thu Jun 27 16:23:14 2019
From: pd@|gd @end|ng |rom gm@||@com (peter dalgaard)
Date: Thu, 27 Jun 2019 16:23:14 +0200
Subject: [Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
In-Reply-To: <03ED7150-6BF7-4E6E-AD78-B6DFACE43A97@gmail.com>
References: <CAFDcVCQOWVr0p2e2HONOy9NvqPdpoAH=HPMOEP65K-N6UjhA2g@mail.gmail.com>
 <67D2E7FE-74E3-4379-B9FB-C4028AB808DF@gmail.com>
 <CAFDcVCTARMfrJRs-KiS5UiSo=SoU8+TgPh_KE+e-zFRR0eCSFQ@mail.gmail.com>
 <23824.41030.156424.792062@stat.math.ethz.ch>
 <CAFDcVCTq-Zc49t9HrdVQB4VHhOtuaoOmpGnx9JsAy+_esene3w@mail.gmail.com>
 <03ED7150-6BF7-4E6E-AD78-B6DFACE43A97@gmail.com>
Message-ID: <6334E748-4121-41E7-9CD4-3F3B2278E8DC@gmail.com>

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



[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
peter dalgaard