From pd@|gd @end|ng |rom gm@||@com  Tue Jun 25 11:48:05 2019
From: pd@|gd @end|ng |rom gm@||@com (peter dalgaard)
Date: Tue, 25 Jun 2019 11:48:05 +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: <73697C70-3117-4432-A1C4-8060446311E9@gmail.com>

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



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