[Bioc-devel] condition tests on vectors with length greater than 1
Kevin RUE
kevinrue67 at gmail.com
Fri Mar 31 19:30:12 CEST 2017
Hi Martin,
My apologies to Tomas Kalibera. I read too fast and missed the part where
you gave him credit for the report.
As I wrote in my previous email, I have started a significant clean-up of
the GOexpress package to improve both the coding standards and the user
experience. I haven't reached the overlap_GO function yet (that uses the
VennDiagram package); however, this is not a core function (I used it for
diagnostic mostly during package development), and I will most likely
drop it altogether before the next release.
Have a great weekend.
Kind regards,
Kevin
On Fri, Mar 31, 2017 at 5:00 PM, Martin Morgan <
martin.morgan at roswellpark.org> wrote:
> 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.R
>> out","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 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.
>
[[alternative HTML version deleted]]
More information about the Bioc-devel
mailing list