[Bioc-devel] condition tests on vectors with length greater than 1
Martin Morgan
martin.morgan at roswellpark.org
Fri Mar 31 18:00:03 CEST 2017
On 03/31/2017 08:21 AM, Kevin RUE wrote:
> Hi Martin,
>
> Thanks a lot for systematically identifying the issue.
>
> Looking at the issue for GOexpress now, I realise that the issue is not
> in GOexpress itself, but actually within the VennDiagram package that
> GOexpress imports. You actually captured that in your report:
>
> "GOexpress","_VennDiagram_","GOexpress.Rcheck/GOexpress-Ex.Rout","VennDiagram::adjust.venn","VennDiagram/R/adjust.venn.R#42 "
>
> In your report, GOexpress is the only Bioconductor in this situation.
> I can (and will) attempt to contact the maintainer of VennDiagram, but
> it just makes me wonder if you have another suggestion to deal with this
> kind of issue (/i.e./, imported packages not conforming to BioC
> guidelines). Is this going to raise Warnings or Errors in BiocCheck ? If
> so, should we ignore such packages altogether, as long as those packages
> raise check issues?
Actually, the report was generated by an R-core member, Tomas Kalibera
and the more extensive version includes all CRAN and Bioc packages. It
reflects a general effort to address these and then make them errors,
rather than warnings -- R CMD check would then fail.
Package dependencies are interesting. They are obviously hugely
important to reduce redundant code, and to fix bugs once in a single
place. But they also carry with them an additional maintenance cost. If
VennDiagram is central to your package, especially if it implements
complicated functionality, then definitely the balance is in favor of
depending on it, and working with the maintainer to address problems
that you discover.
The Bioc requirements are really about your package, especially
enforcing adequate documentation.
Martin
>
> As a side note, I have actually already been going through GOexpress to
> prepare a new major version of GOexpress (2.x.x) that will introduce S4
> where relevant, and break down large functions into more sensibly-sized
> steps. However, most likely that won't be ready for the next BioC release.
>
> Best,
> Kevin
>
>
> On Wed, Mar 29, 2017 at 10:44 PM, Martin Morgan
> <martin.morgan at roswellpark.org <mailto:martin.morgan at roswellpark.org>>
> wrote:
>
> On 03/29/2017 05:37 PM, Martin Morgan wrote:
>
> Bioc developers,
>
> R emits a warning when a condition test has length > 1
>
> $ R --vanilla
>
> if (letters == "a") TRUE
>
> [1] TRUE
> Warning message:
> In if (letters == "a") TRUE :
> the condition has length > 1 and only the first element will
> be used
>
> These are almost always programming errors.
>
> R-3-4-branch and R-devel can be configured to report such errors, as
> described on the help page for, e.g., `?"if"`
>
> $ _R_CHECK_LENGTH_1_CONDITION_=TRUE R --vanilla
>
> if (letters=="a") TRUE
>
> Error in if (letters == "a") TRUE : the condition has length > 1
>
> The attached file (thanks to Tomas Kalibera) summarizes Bioconductor
> code that triggers this type of error.
>
>
> Second try on the attachment
>
> r = read.csv("long-conditional-bioc.txt")
>
>
>
> If you maintain one of these packages (appearing in either column),
> please address the error. And of course as a developer, please avoid
> making the error in the future!
>
> r = read.csv("long-conditional-bioc.csv")
> r[, c("FailedPackage", "Srcref")]
>
> FailedPackage Srcref
> 1 biovizBase biovizBase/R/crunch-method.R#295
> 2 branchpointer branchpointer/R/makeRegions.R#41
> 3 Cardinal Cardinal/R/spatial.R#57
> 4 debrowser debrowser/R/heatmap.R#38
> 5 DMRcate DMRcate/R/DMR.plot.R#13
> 6 exomePeak exomePeak/R/RMT.R#119
> 7 fabia fabia/R/fabia.R#1504
> 8 GenomeInfoDb GenomicFeatures/R/TxDb-class.R#377
> 9 Glimma Glimma/R/hexcol.R#32
> 10 GOexpress VennDiagram/R/adjust.venn.R#42
> 11 GUIDEseq GUIDEseq/R/offTargetAnalysisOfPeakRegions.R#95
> 12 hapFabia hapFabia/R/methods-IBDsegmentList-class.R#110
> 13 MassArray MassArray/R/convControl.R#26
> 14 methylPipe methylPipe/R/Allfunctions.R#635
> 15 NOISeq NOISeq/R/biodetection.plot.R#157
> 16 pathview pathview/R/geneannot.map.R#31
> 17 phyloseq phyloseq/R/multtest-wrapper.R#101
> 18 rHVDM rHVDM/R/measurementerrorHVDM.R#23
> 19 SEPA SEPA/R/truetimevisualize.R#28
> 20 SPLINTER SPLINTER/R/main_splinter.R#817
>
> As an example the GenomeInfoDb package (row 8) has this complete
> record
>
> FailedPackage "GenomeInfoDb"
> IfPackage "GenomicFeatures"
> File "GenomeInfoDb.Rcheck/GenomeInfoDb-Ex.Rout"
> Function "S4 Method seqlevels<-:GenomeInfoDb defined in
> namespace
> GenomicFeatures with signature TxDb has this body."
> Srcref "GenomicFeatures/R/TxDb-class.R#377 "
>
> The problem was from
>
> GenomicFeatures/R/TxDb-class.R#377
>
> which has
>
> mode <- GenomeInfoDb:::getSeqlevelsReplacementMode(value,
> x_seqlevels0)
> if (mode == -2L) {
>
> I looked at
>
> GenomeInfoDb:::getSeqlevelsReplacementMode
>
> function (new_seqlevels, old_seqlevels)
> {
> ...
>
> and saw that its code returns a vector with length > 1 intentionally
> under some specific circumstances. Also, all other uses of the
> return
> value of this function (in the GenomeInfoDb and GenomicFeatures
> package)
> test for identity of the return value via `identical()`, which
> is always
> a scalar. This suggests the fix
>
> GenomicFeatures$ svn diff R/TxDb-class.R
> Index: R/TxDb-class.R
> ===================================================================
> --- R/TxDb-class.R (revision 127829)
> +++ R/TxDb-class.R (working copy)
> @@ -374,7 +374,7 @@
> ## detect the situation where the user intention is to
> subset the
> "real"
> ## seqlevels.
> mode <- GenomeInfoDb:::getSeqlevelsReplacementMode(value,
> x_seqlevels0)
> - if (mode == -2L) {
> + if (identical(mode, -2L)) {
> ## "subsetting of the real seqlevels" mode
> x$user_seqlevels <- value
> x$user2seqlevels0 <- match(value, x_seqlevels0)
>
> I did do some more digging around.
>
> From the report, the failure was detected while running the
> example page
> script generated while checking GenomeInfoDb.
>
> GenomeInfoDb.Rcheck/GenomeInfoDb-Ex.Rout
>
> So I generated this
>
> R CMD build --no-build-vignettes GenomeInfoDb
> R CMD check GenomeInfoDb_1.27.11.tar.gz
>
> and processed it to find the error
>
> _R_CHECK_LENGTH_1_CONDITION_=TRUE R -f
> GenomeInfoDb.Rcheck/GenomeInfoDb-Ex.R
>
> It failed while running the example on the help page with name
> "seqlevels-wrapper"
>
> ### Name: seqlevels-wrappers
> ### Title: Convenience wrappers to the seqlevels() getter
> and setter
> ### Aliases: seqlevels-wrappers keepSeqlevels dropSeqlevels
>
> renameSeqlevels
>
> ### restoreSeqlevels standardChromosomes
> keepStandardChromosomes
> ### Keywords: methods utilities
>
> ### ** Examples
>
> ...
>
> renameSeqlevels(txdb, sub("chr", "CH", seqlevels(txdb)))
>
> Error in if (mode == -2L) { : the condition has length > 1
> Calls: renameSeqlevels -> seqlevels<- -> seqlevels<-
> Execution halted
>
> I then could get a relatively simple reproducible example in R with
>
> $ _R_CHECK_LENGTH_1_CONDITION_=TRUE R --vanilla
>
> suppressPackageStartupMessages(library(GenomeInfoDb))
> example("seqlevels-wrappers")
>
>
> After installing the updated GenomicFeatures package, the error
> did not
> occur. Likewise, running GenomeInfoDb-Ex.R did not generate any
> errors.
>
> Martin
>
>
> This email message may contain legally privileged and/or
> confidential
> information. If you are not the intended recipient(s), or the
> employee
> or agent responsible for the delivery of this message to the
> intended
> recipient(s), you are hereby notified that any disclosure, copying,
> distribution, or use of this email message is prohibited. If
> you have
> received this message in error, please notify the sender
> immediately by
> e-mail and delete this email message from your computer. Thank you.
> _______________________________________________
> Bioc-devel at r-project.org <mailto:Bioc-devel at r-project.org>
> mailing list
> https://stat.ethz.ch/mailman/listinfo/bioc-devel
> <https://stat.ethz.ch/mailman/listinfo/bioc-devel>
>
>
>
> This email message may contain legally privileged and/or
> confidential information. If you are not the intended recipient(s),
> or the employee or agent responsible for the delivery of this
> message to the intended recipient(s), you are hereby notified that
> any disclosure, copying, distribution, or use of this email message
> is prohibited. If you have received this message in error, please
> notify the sender immediately by e-mail and delete this email
> message from your computer. Thank you.
>
> _______________________________________________
> Bioc-devel at r-project.org <mailto:Bioc-devel at r-project.org> mailing list
> https://stat.ethz.ch/mailman/listinfo/bioc-devel
> <https://stat.ethz.ch/mailman/listinfo/bioc-devel>
>
>
This email message may contain legally privileged and/or...{{dropped:2}}
More information about the Bioc-devel
mailing list