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.mor...@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.mor...@roswellpark.org <mailto:martin.mor...@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@r-project.org <mailto:Bioc-devel@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@r-project.org <mailto:Bioc-devel@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]] _______________________________________________ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel
