[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
Martin Maechler
m@ech|er @end|ng |rom @t@t@m@th@ethz@ch
Thu Jun 27 17:15:50 CEST 2019
>>>>> peter dalgaard
>>>>> on Thu, 27 Jun 2019 16:23:14 +0200 writes:
> 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
I've also have mused over that question...
and I had assumed some difference in the case the original
omittedSig contains NAs ... but that's NOT true actually, see:
> sign2 <- signatures <- LETTERS
> omittedSig <- LETTERS < "K"
> omittedSig[c(8,18)] <- NA # now have an omittedSig with {T, F, NA}
> iSig <- seq_along(omittedSig)[omittedSig]
> sign2[iSig] <- "missing"
> signatures[omittedSig] <- "missing"
> identical(sign2, signatures)
[1] TRUE
>
so I still don't see the case where it makes a difference.
Martin
>> On 25 Jun 2019, at 09:44 , peter dalgaard <pdalgd using 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 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
>>
>>
>>
>>
>>
>>
>>
>>
>>
> --
> 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