From m@ech|er @end|ng |rom @t@t@m@th@ethz@ch  Thu Jun 27 17:15:50 2019
From: m@ech|er @end|ng |rom @t@t@m@th@ethz@ch (Martin Maechler)
Date: Thu, 27 Jun 2019 17:15:50 +0200
Subject: [Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
In-Reply-To: <6334E748-4121-41E7-9CD4-3F3B2278E8DC@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>
 <6334E748-4121-41E7-9CD4-3F3B2278E8DC@gmail.com>
Message-ID: <23828.56742.426539.104574@stat.math.ethz.ch>

>>>>> 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 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
Martin Maechler