[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