DEComplexDisease

[menggf]

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

• Repository: https://github.com/menggf/DEComplexDisease

Confirm the following by editing each check box to '[x]'

• [x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.

• [x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.

• [x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.

• [x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.

• [x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

I am familiar with the essential aspects of Bioconductor software management, including:

• [x] The 'devel' branch for new packages and features.
• [x] The stable 'release' branch, made available every six months, for bug fixes.
• [x] Bioconductor version control using Subversion (optionally via GitHub).

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

[bioc-issue-bot]

Hi @menggf

Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Package: DEComplexDisease
Type: Package
Title: A tool for differential expression analysis and DEGs based investigation to complex diseases by bi-clustering analysis
Version: 0.99.0
Author: Guofeng Meng
Maintainer: Guofeng Meng <menggf@gmail.com>
Description: It is designed to find the differential expressed genes (DEGs) for complex disease, which is characterized by the heterogeneous genomic expression profiles. Different from the established DEG analysis tools, it does not assume the patients of complex diseases to share the common DEGs. By applying a bi-clustering algorithm, DECD finds the DEGs shared by as many patients. In this way, DECD describes the DEGs of complex disease in a novel syntax, e.g. a gene list composed of 200 genes are differentially expressed in 30% percent of studied complex disease. Applying the DECD analysis results, users are possible to find the patients affected by the same mechanism based on the shared signatures.
License: GPL-3
LazyData: TRUE
Dependence:
    R (>= 3.3.1)
Imports:
    Rcpp (>= 0.12.7),
    DESeq2,
    edgeR,
    ComplexHeatmap,
Suggests:
  knitr
LinkingTo: Rcpp
biocViews: DifferentialExpression, BiomedicalInformatics, Clustering, SystemsBiology, Transcriptomics, GeneExpression
RoxygenNote: 6.0.1
VignetteBuilder: knitr

[bioc-issue-bot]

Your package has been approved for building. Your package is now submitted to our queue.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170822045958.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

2291fef Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170822225507.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

399df8e Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170823045612.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

0e8313a Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170823054514.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

0c8ff57 Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170823210634.html

[nturaga]

Review DEComplexDisease

R CMD build

• ok

R CMD INSTALL

• ok

DESCRIPTION

• ok.

• DifferntialExpression as a biocView.

NAMESPACE

• Add

  importFrom("grDevices", "rainbow")
  importFrom("graphics", "abline", "axis", "legend", "lines", "mtext",
           "par", "plot", "points", "text")
  importFrom("methods", "is")
  importFrom("stats", "fisher.test", "kmeans", "lm", "loess", "pnbinom",
           "pnorm", "predict", "quantile", "sd")

man

• ok.

vignette

• Good vignette as far as information.

• Just keep the Rmd file, get rid of everything else.

• Make sure your code chunks evaluate, use

  {r, eval=TRUE}

• Use BiocFileCache, to download.file("https://...") in vignette. FYI, link is dead.

C++

• Instead of sig.cpp, l:530

 cout<<"DEG: "<< st <<":"<< num_w << "\t" << x << "\t" << y << "\t" << overlap << "\t" <<         output1 <<endl;

use below or remove the above code.

Rcpp::Rcout << .....

R

• util.R is totally unreadable code. There is no documentation, and the variable names are poorly constructed.

• Please use formatR to format your code(https://cran.rstudio.com/web/packages/formatR/index.html). Eg: Plot.deg, the indentation is off, which makes your code very hard to read.

  Make this change in all your .R files.

• instead of

  88                 bi=sapply(1:length(p), function(w){
  89                         if(p[w] <= mycutoff[w] )
  90                             return(1)
  91                         if(1- p[w] <= mycutoff[w] )
  92                             return(-1)
  93                         return(0);
  94                     });

Use:

bi = integer(length(p))
bi[ p <= mycutoff ] = 1
bi[ 1 - p <= mycutoff ] = -1

• Use BiocParallel::bplapply instead of mclapply everywhere.

• Avoid

  module.screen.R
  72                 p=sapply(1:length(c.feature), function(i){

  Use seq_along(c.feature) instead and similar. Wherever you see, 1:length(blah), please use seq_along or seq_len as needed.

  Similarly in module.compare.R, l 69,

   for(i in 1:n1){
       ges1=res.module1[[mod1[i]]][[type]][["genes"]];
       for(j in 1:n2){
           ges2=res.module2[[mod2[j]]][[type]][["genes"]];
           olp2[i,j]=length(intersect(ges1, ges2));
           olp4[i,j]=olp2[i,j]/max(len3[i], len4[j])
       }
   }

  Use seq_len(n1) and seq_len(n2) as well. Also, check, module.overlap.R, module.screen.R for the same issue.

  if you want to iterate from the 10th element onwards eg: in module.scree.R, l 56

  tail(seq_along(add.patients),-9)

• In summarize.module.R,

pas=pas[pas != "decd.input" & pas != "decd.speicific" & pas != "decd.clustering"];

You have a typo for specific.

• Closely examine ALL iterations (for, lapply(), sapply(), apply(), ...) for opportunities to "vectorize", i.e., replace many calls to R functions with one call, as in the following, from module.modeling.R:118

newsc=apply(sub.deg,2, function(x) length(which(x== new.seed))/new.n)

has ncol(sub.deg) calls to length(which(.)), ==, and /, whereas

newsc = colSums(sub.deg == new.seed) / new.n

has just 1 call to colSums(), ==, and /

Do this everywhere.

• Replacing iterations with vectorization will greatly speed up your code. Maybe with vectorization it will NOT be necessary to use parallel evalation?

• summarize.module.R: use a 'helper' function to simplify these sapply(), e.g.,

    fun = function(x, elt1, elt2) res.module[[x]][[elt1]][[elt2]]
    sc1 = unlist(sapply(pas, fun, "max.genes", "sc"))
    ...

but these "nested lists" seem to be a problem throughout your code, e.g., module.overlap.R:50. A better approach is to make your data 'tidy' when you create it, instead of trying to manipulate complicated nested lists. See http://vita.had.co.nz/papers/tidy-data.html.

[menggf]

Dear @nturaga , Thank you for the good comments. Please give more time to do the modification. I hope to submit the new submission before the end of next week (Sep. 8).

Best Regards, Guofeng,

[nturaga]

Hi @menggf

Let me know when you are ready for the next review.

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

01c102d Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170917000855.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

bbcdfef Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20170917013738.html

[menggf]

Hi @nturaga ,

I corrected nearly all the mentioned problems and some other problems. Best, Guofeng,

[nturaga]

Hi @menggf

The vignette doesn't evaluate because the R tags are not used. I need to be able to evaluate the vignette before i'm able to review your code again.

https://www.bioconductor.org/developers/package-guidelines/#vignettes

Please follow the vignette guidelines, also check other bioconductor packages for vignettes if you are not sure how to write them.

[nturaga]

• NAMESPACE edited by hand, instead of adding @importFrom at the appropriate places in the documentation.

  Please check http://r-pkgs.had.co.nz/namespace.html,

  "Roxygen2 makes NAMESPACE tidy. No matter how many times you use @importFrom foo bar you’ll only get one importFrom(foo, bar) in your NAMESPACE. This makes it easy to attach import directives to every function that need them, rather than trying to manage in one central place."

• Vignette doesn't evaluate because the R tags are not used. The reviewers need to be able to evaluate the vignette, the package cannot go through without this.

  https://www.bioconductor.org/developers/package-guidelines/#vignettes

  Please follow the vignette guidelines, also check other bioconductor packages.

  e.g

  ```{r, eval=TRUE} hc=hclust(dist(t(exp.brca[, cl==0]))) ```

• Remove commented out code

[nturaga]

@menggf Please provide a summary of the changes you have made, and I will review your package again.

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

d16c9f6 0.99.13

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ABNORMAL". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171001100102.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

aa5453c 0.99.15

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ABNORMAL". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171001103313.html

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171005104841.html

[nturaga]

@menggf Please respond with the changes you have made. I will close the issue if there is no response by Oct 19th (2 weeks from this message).

[menggf]

Hi, @nturaga ,

Sorry for the laggard response for the reason that I am on a holiday.

Most of the changes are done by following your comments, including but not limited to: (1) update the package dependence in Description.txt and Roxygen2 document (2) Add comments to util.R (3) Vectorize some "for" and "apply" statement, including all the mentioned comments and several new ones. (4) Replace mcapply with BiocParallel::bplapply (5) Use seq_along or seq_len to generate sequential vectors (6) Format the code usingformatR

Not changed: vignette code chunks evaluate: the vignette need huge dataset to output the same results and it is not feasible.

Best Regards, Guofeng,

[nturaga]

Hi @menggf

• It's Depends, not Dependence for the DESCRIPTION file tag. Please correct this.

• The vignette is important for a bioconductor package, it needs to be evaluated. Can't you make the data set smaller for the vignette? There are helpful guidelines here http://bioconductor.org/developers/package-guidelines/#vignettes. If a user cannot run your vignette in a meaningful way, then they will have trouble following your package or using it.

[menggf]

Hi, @nturaga ,

We completely understand your concerns and also believe it important to make vignette to be evaluated. Indeed, we have attempted to select the samples and genes to generate vignette data with acceptable size. However, the analysis results were not right and it would mislead readers. We believe that it is completely not acceptable to generate improper results while just to make sure it technologically sounds.

Best Regards,

Guofeng,

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

6a3e0ca Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171019203240.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

ea2297e Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171020054222.html

[mtmorgan]

The purpose of the vignette is to illustrate package use, not to replicate a comprehensive analysis. (almost) NO OTHER BIOCONDUCTOR PACKAGE AUTHOR has failed to provide a meaningful vignette with most code chunks fully evaluated. It is a REQUIREMENT of Bioconductor packages to have such a vignette, so you must either identify a suitable approach or find other venues for distributing your package.

A typical problem is that code is inefficient, the nested 'for' loops in your code make it seem likely that you are using iteration rather than vectorization. Two useful resources on this are http://bioconductor.org/developers/how-to/efficient-code/#vectorize and https://github.com/Bioconductor/Contributions/issues/516#issuecomment-337962416 .

[lawremi]

Technical soundness needs to take precedence over whether the results are interesting/meaningful. One compromise is demonstrate an uninteresting but working analysis, and then load cached results from a bigger one to demonstrate downstream steps.

[menggf]

@nturaga , @mtmorgan , @lawremi , If I did not misunderstand it, R package is technically evaluated in two ways: (1) \examples{} in *.Rd and (2) vignette chunks. We use the former as technical evaluation for every function of this package. All the examples have been set runnable and they will generate meaningless results. We use the later to demonstrate package usage and explain the users how to interpret the results. It needs users to download dataset from third-party websites. So, we set it to be not evaluated.

If you still believe it insufficient, I can replace the vignette with example codes. From the view of supporting users, I don't want to do so.

Please give me your feedback.

[mtmorgan]

In terms of testing, a modern and comprehensive R package would use formal unit tests to evaluate the logic and implementation of functions and other granular components. R does not really have formal support for 'integration' tests (do the parts of the package work together as advertised?); the vignette is the most important document for this purpose, followed by a lesser extent by examples.

R packages without formal unit tests rely on examples and vignettes for all testing, both 'unit' and 'integration'.

In terms of documentation, examples on man pages are meant to guide the user consulting the man page in how to use the function(s) or data documented on that man page. Vignettes are meant to provide an integrated use of the package over all, including where appropriate how it might be used with other parts of the Bioconductor system.

The vignette therefore serves two essential purposes, as a test of the code over all, and as integrative documentation.

Your static code chunks disable the integration and general test functionality of the vignette. Your vignette almost certainly covers code paths not covered in your examples, in particular when your package is built on different platforms and when there are changes in the code of your package or the packages you depend on. I can only say from observing the Bioconductor build system for the last 12 years that vignettes are invaluable in revealing technical faults in code.

Your static code chunks also change an actual analysis into a kind of fiction, where the pictures of code might have worked at one time or on one computer, but do not work now or on another computer. As a simple case in point, I tried to follow your vignette. Here's the first line I evaluated

> download.file("https://superb-sea2.dl.sourceforge.net/project/decd/decd_data.rda",
              "decd_data.rda");
trying URL 'https://superb-sea2.dl.sourceforge.net/project/decd/decd_data.rda'
Error in download.file("https://superb-sea2.dl.sourceforge.net/project/decd/decd_data.rda",  : 
  cannot open URL 'https://superb-sea2.dl.sourceforge.net/project/decd/decd_data.rda'
In addition: Warning message:
In download.file("https://superb-sea2.dl.sourceforge.net/project/decd/decd_data.rda",  :
  cannot open URL 'https://downloads.sourceforge.net/project/decd/decd_data.rda?download&failedmirror=superb-sea2.dl.sourceforge.net': HTTP status was '404 Not Found'

I did eventually find https://sourceforge.net/projects/decd/files/?source=navbar where there is a file, but it's called 'input0.rda'. So as a user I can now get nothing with confidence out of your vignette. I downloaded the file anyway, and got to the next code chunk, where I did

> hc=hclust(dist(t(exp.brca[, cl==0])))
Error in t(exp.brca[, cl == 0]) : object 'cl' not found

Arrgh! Also, this matrix + column metadata format is ideally suited to ExpressionSet or SummarizedExperiment, and I don't understand why it is not in that format.

Replacing your vignette with a collation of examples undermines the documentation role of vignette as an illustration of integrative package use. It also removes the integrative testing component, and likely reduces the amount of code covered by the functions evaluated in the package.

In short, your vignette must have fully evaluated code chunks. The code chunks should take the reader through a real work flow. With some work, I am fully confident that you can identify an appropriate subset of data that is computationally tractable, statistically meaningful, and biologically interesting.

[menggf]

@mtmorgan I can understand the concerns. Let me find some balance. Update may take one week,

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

ae2c69f Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171106093248.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

32ca70c Update DESCRIPTION

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171106100119.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

90e1733 0.99.25

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171107085009.html

[bioc-issue-bot]

Received a valid push; starting a build. Commits are:

0d725f5 0.99.27

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/DEComplexDisease_buildreport_20171107092418.html

[menggf]

@mtmorgan, @nturaga Please have a look.

[nturaga]

Hi @menggf I'll take a look and get back to you