From henr|k@bengt@@on @end|ng |rom gm@||@com  Fri Jun 28 01:00:39 2019
From: henr|k@bengt@@on @end|ng |rom gm@||@com (Henrik Bengtsson)
Date: Thu, 27 Jun 2019 16:00:39 -0700
Subject: [Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
In-Reply-To: <23828.56742.426539.104574@stat.math.ethz.ch>
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>
 <23828.56742.426539.104574@stat.math.ethz.ch>
Message-ID: <CAFDcVCSoHT-GxRyw-bncfnbzbG19t9sUpvPnpg=Edz_j35DiFA@mail.gmail.com>

Using:

untrace(methods::conformMethod)
at <- c(12,4,3,2)
str(body(methods::conformMethod)[[at]])
## language omittedSig <- omittedSig && (signature[omittedSig] != "missing")
cc <- 0L
trace(methods::conformMethod, tracer = quote({
  cc <<- cc + 1L
  print(cc)
  if (cc == 31) {  ## manually identified
    untrace(methods::conformMethod)
    trace(methods::conformMethod, at = list(at), tracer = quote({
      str(list(signature = signature, mnames = mnames, fnames = fnames))
      print(ls())
      try(str(list(omittedSig = omittedSig, signature = signature)))
    }))
  }
}))
loadNamespace("oligo")

gives:

Untracing function "conformMethod" in package "methods"
Tracing function "conformMethod" in package "methods"
Tracing conformMethod(signature, mnames, fnames, f, fdef, definition)
step 12,4,3,2
List of 3
 $ signature: Named chr [1:4] "TilingFeatureSet" "ANY" "ANY" "array"
  ..- attr(*, "names")= chr [1:4] "object" "subset" "target" "value"
  ..- attr(*, "package")= chr [1:4] "oligoClasses" "methods" "methods" "methods"
 $ mnames   : chr [1:2] "object" "value"
 $ fnames   : chr [1:4] "object" "subset" "target" "value"
 [1] "f"          "fdef"       "fnames"     "fsig"       "imf"
 [6] "method"     "mnames"     "omitted"    "omittedSig" "sig0"
[11] "sigNames"   "signature"
List of 2
 $ omittedSig: logi [1:4] FALSE TRUE TRUE FALSE
 $ signature : Named chr [1:4] "TilingFeatureSet" "ANY" "ANY" "array"
  ..- attr(*, "names")= chr [1:4] "object" "subset" "target" "value"
  ..- attr(*, "package")= chr [1:4] "oligoClasses" "methods" "methods" "methods"
Error in omittedSig && (signature[omittedSig] != "missing") :
  'length(x) = 4 > 1' in coercion to 'logical(1)'
Error: unable to load R code in package 'oligo'

This is with:

sessionInfo()
R version 3.6.0 Patched (2019-06-23 r76734)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.2 LTS

Matrix products: default
BLAS:   /home/hb/software/R-devel/R-3-6-branch/lib/R/lib/libRblas.so
LAPACK: /home/hb/software/R-devel/R-3-6-branch/lib/R/lib/libRlapack.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.1                  affxparser_1.56.0
 [3] XVector_0.24.0              splines_3.6.0
 [5] GenomicRanges_1.36.0        BiocGenerics_0.30.0
 [7] zlibbioc_1.30.0             IRanges_2.18.1
 [9] bit_1.1-14                  BiocParallel_1.18.0
[11] lattice_0.20-38             foreach_1.4.4
[13] blob_1.1.1                  GenomeInfoDb_1.20.0
[15] tools_3.6.0                 SummarizedExperiment_1.14.0
[17] parallel_3.6.0              grid_3.6.0
[19] Biobase_2.44.0              ff_2.2-14
[21] DBI_1.0.0                   iterators_1.0.10
[23] oligoClasses_1.46.0         matrixStats_0.54.0
[25] digest_0.6.19               bit64_0.9-7
[27] preprocessCore_1.46.0       affyio_1.54.0
[29] Matrix_1.2-17               GenomeInfoDbData_1.2.1
[31] BiocManager_1.30.4          codetools_0.2-16
[33] S4Vectors_0.22.0            bitops_1.0-6
[35] RCurl_1.95-4.12             memoise_1.1.0
[37] RSQLite_2.1.1               DelayedArray_0.10.0
[39] compiler_3.6.0              Biostrings_2.52.0
[41] stats4_3.6.0

/Henrik

On Thu, Jun 27, 2019 at 8:16 AM Martin Maechler
<maechler at stat.math.ethz.ch> wrote:
>
> >>>>> 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
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel



[Rd] methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error
Henrik Bengtsson