signeR - Githubissues

rvalieris commented 8 years ago

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

Repository: https://github.com/rvalieris/signeR

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] 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 commented 8 years ago

Hi @rvalieris

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: signeR
Type: Package
Title: signeR 
Version: 0.99.0
Author: Rafael Rosales, Rodrigo Drummond, Renan Valieris, Israel Tojal da Silva
Maintainer: Renan Valieris <renan.valieris@cipe.accamargo.org.br>
Description: The signeR package provides an empirical Bayesian approach to mutational signature discovery. It is designed to analyze single nucleotide variaton (SNV) counts in cancer genomes, but can also be applied to other features as well. Functionalities to characterize signatures or genome samples according to exposure patterns are also provided. 
License: GPL-3
Imports: BiocGenerics, Biostrings, BSgenome, class, grDevices, GenomicRanges, nloptr, NMF, R.methodsS3, R.oo, tensorA, VariantAnnotation
LinkingTo: Rcpp, RcppArmadillo
SystemRequirements: C++11
NeedsCompilation: yes
ByteCompile: TRUE
biocViews: GenomicVariation, SomaticMutation, StatisticalMethod, Visualization
Suggests: knitr, rtracklayer, base64
VignetteBuilder: knitr

bioc-issue-bot commented 8 years ago

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 commented 8 years ago

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/signeR_buildreport_20160616174056.html

bioc-issue-bot commented 8 years ago

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

744c9e9 added stats,graphics,utils to namespace 109b37c added new parameter for Classify

bioc-issue-bot commented 8 years ago

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/signeR_buildreport_20160617112017.html

rvalieris commented 8 years ago

so it seems the windows build is failling because it can't find LAPACK/BLAS symbols. is there something I can do to fix this ?

edit: nevermind I figured it out.

bioc-issue-bot commented 8 years ago

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

01297ad added armadillo dependencies to Makevars

bioc-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

bioc-issue-bot commented 8 years ago

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

585ea2e new parameter EM_eval

bioc-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

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

67275f7 bump version

rvalieris commented 8 years ago

Sorry for these last minute commits, we were syncing the docs with the paper. This should be the last one, I will wait for your review now.

bioc-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

bioc-issue-bot commented 8 years ago

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

b7f9c65 docs refactor 6d399c2 bug fix: set background value for null rows/column...

bioc-issue-bot commented 8 years ago

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/signeR_buildreport_20160621172955.html

bioc-issue-bot commented 8 years ago

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

43357d9 changed wording on vignette

bioc-issue-bot commented 8 years ago

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/signeR_buildreport_20160622105709.html

bioc-issue-bot commented 8 years ago

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

feeb511 refactor vignette, removing most of eval=FALSE

bioc-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

bioc-issue-bot commented 8 years ago

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

a18aa0b removed "we" from vignette

bioc-issue-bot commented 8 years ago

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/signeR_buildreport_20160624121501.html

bioc-issue-bot commented 8 years ago

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

e7f0f32 added a progressbar to signeR()

LiNk-NY commented 8 years ago

Hi @rvalieris, Thank you for submitting your package to Bioconductor. I am reviewing your package. You will be getting a review shortly.

Regards, Marcel

rvalieris commented 8 years ago

Hello Marcel, thank you for the heads up. I'm looking forward to it.

LiNk-NY commented 8 years ago

Hi @rvalieris, Thank you again for your submission. Our review is below. Please reach out to me if you have any questions.

\dontrun{

grep dontrun man/* man/classify.Rd:\dontrun{ man/diffexp.Rd:\dontrun{ man/genMatrix.Rd:\dontrun{ man/plots.Rd:\dontrun{

Please remove the ‘dontrun’ sections or explain why you need them there.

Long functions: eBayesNMF() (R/cpp_eBayesNMF.R, line 5): 186 lines anonymous.185() (R/SignExp.R, line 185): 167 lines signeR() (R/signeR.R, line 1): 163 lines

Long lines: 230 lines (11%) are > 80 characters

Indentations: 612 lines (30%) are not indented with a multiple of 4 spaces

Looking at some R scripts, there are no indentations in the singeR.R script and it seems like different contributors wrote different scripts with their own formatting/coding conventions. Please keep the formatting consistent.

library calls A few scripts use library to load packages. Please remove all library calls in the code. These packages should be in depends or imports:

grep library * cpp_eBayesNMF.R:library(tensorA) cpp_eBayesNMF.R:library(NMF) cpp_eBayesNMF.R:library(nloptr) genMatrix.R:library(VariantAnnotation) SignExp.R:library(R.methodsS3) SignExp.R:library(R.oo) SignExp.R:library(class) SignExp.R:library(nloptr)

SignExp class should be S4 if you need a formal class; also need getters and setters as necessary

only S3 methods are those that previously exist; otherwise just use regular functions or S4 methods

For example plot() is an S3 generic with methods. Type plot and hit TAB to see the methods -

plot plot plot.design plot.function plot.spec.coherency plot.stepfun plot.window plotDispEsts plotPCA plot.default plot.ecdf plot.new plot.spec.phase plot.ts plot.xy plotMA

The name after the dot '.' is the name of the class plot dispatches on.

These should be regular functions or S4 methods:

grep setMethodS3 * SignExp.R:setMethodS3("Normalize",class="SignExp",function(this,...){ SignExp.R:setMethodS3("Reorder",class="SignExp",function(this,ord,...){ SignExp.R:setMethodS3("Average_sign",class="SignExp",function(this,normalize=TRUE,...){ SignExp.R:setMethodS3("Median_sign",class="SignExp",function(this,normalize=TRUE,...){ SignExp.R:setMethodS3("Average_exp",class="SignExp",function(this,normalize=TRUE,...){ SignExp.R:setMethodS3("Median_exp",class="SignExp",function(this,normalize=TRUE,...){ SignExp.R:setMethodS3("Paths",class="SignExp",function(this,file_suffix="plot.pdf",plots_per_page=4,...){ SignExp.R:setMethodS3("SignPlot",class="SignExp",function(this,plotfile="Signature_plot.pdf",pal='bcr1', SignExp.R:setMethodS3("ExposureBoxplot",class="SignExp",function(this,plotfile="Exposure_boxplot.pdf", col='tan2', SignExp.R:setMethodS3("SignHeat",class="SignExp",function(this,plotfile="Signature_heatmap.pdf",nbins=20,...){ SignExp.R:setMethodS3("DiffExp",class="SignExp",function(this,labels,method="kw",contrast="all",quant=0.5, SignExp.R:setMethodS3("Classify",class="SignExp",function(this,labels,method="knn",k=3,plotfile="Classification_barplot.pdf",

plots should return a plot; fine to have an option to write to a file but that should not be the default. General rule is that file creation is optional and an explicit call is required by the user.

Please avoid giving arguments names like this.

Looking forward to seeing the appropriate changes.

Regards, MR

bioc-issue-bot commented 8 years ago

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

34e0f31 major code refactor * reformatted all code indent...

rvalieris commented 8 years ago

Hello Marcel,

sorry for the delay, we just finished making the changes.

dontrun

The problem here was that to run these functions it is required the return value of signeR(), which is a function that takes a while to run, depending on the data input.

To solve this I added some mock data, now the functions are run, and its fast too.

code style issues

We have gone through all code and reformated it.

library calls

These have been removed.

SignExp class

We decided to turn SignExp into a S4 class, methods have also been converted.

plots issue

I understand, we modified all methods that generate plots to use the current device.

bioc-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

bioc-issue-bot commented 8 years ago

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

7d89105 * new plot method ExposureHeat * exposed other Sig...

bioc-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

LiNk-NY commented 8 years ago

Hi @rvalieris, Sorry about the delay. Thank you for making those changes. Your package is close to approval. I am currently building your package locally and it is taking a long time on "creating vignettes...". Do you know why that is?

Marcel

rvalieris commented 8 years ago

Hello Marcel,

Yes, sorry I should have mentioned that, the vignette executes a full workflow of the package, so it should take about 30min~1hour to complete successfully.

On Aug 19, 2016 12:50 PM, "Marcel Ramos" notifications@github.com wrote:

Hi @rvalieris https://github.com/rvalieris, Sorry about the delay. Thank you for making those changes. You package is close to approval. I am currently building your package locally and it is taking a long time. Do you know why that is?

Marcel

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/24#issuecomment-241056196, or mute the thread https://github.com/notifications/unsubscribe-auth/AGBKSJT-TVlo0N75DtanLlAASRPnWDDeks5qhdDDgaJpZM4I3zyE .

LiNk-NY commented 8 years ago

Hi @rvalieris, We can't have packages that take that long a time to check. Our build system has thousands of packages that will require bi-nightly builds.

Please shorten the time it takes to check your package to less than 5 minutes.
Your package also is about 11 Mb big. Please reduce the size of the data in the package, it should occupy less than 4 Mb on disk.

It seems like you're okay on the size. You just have some big files inside of your .git folder. Update: Actually your src folder is quite big:

6.2M signeR/src

I meant check not build.

rvalieris commented 8 years ago

okay, I didn't knew for sure if 5 minutes included vignette build time, I will work on shortening it.

but the compiled package should be about 1.6Mb, there shouldn't be any big files in src either, I guess there are compiled object files it your build tree ?

LiNk-NY commented 8 years ago

Hi @rvalieris, Update: Yes, it includes the time it takes to run all the examples in the vignette and not the actual vignette build. Currently, R CMD check --no-build-vignettes signeR_0.99.11.tar.gz takes real 13m27.803s via time. That has to be shortened.

With respect to package size, I was looking at the github checkout rather than the built package. Once built, the package conforms to the size guidelines :+1: (although it would also be good to not upload compiled files to github).

bioc-issue-bot commented 8 years ago

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

2933931 shortened vignette execution time by reducing the ...

rvalieris commented 8 years ago

Hi Marcel,

with this latest commit, the time of both R CMD build and R CMD check --no-build-vignettes should be under 5 minutes.

bioc-issue-bot commented 8 years ago

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/signeR_buildreport_20160822170823.html

LiNk-NY commented 8 years ago

Hi @rvalieris, I just checked your package and it's still taking quite a while to check. time ~ 14m8.754ds

bioc-issue-bot commented 8 years ago

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

298456e reduced execution time on the examples

rvalieris commented 8 years ago

Sorry Marcel, I forgot to reduce the execution times on the manual examples, this commit reduces that.

now on my machine check --no-build-vignettes takes:

real    118.81s
user    113.62s
sys 3.71s
cpu 98%

bioc-issue-bot commented 8 years ago

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/signeR_buildreport_20160829150833.html

LiNk-NY commented 8 years ago

The Windows machine has an error:

Error in loadNamespace(i, c(lib.loc, .libPaths()), versionCheck = vI[[i]]) : 
  there is no package called 'codetools'

@mtmorgan Do you know if the codetools error is an SPB quirk?

mtmorgan commented 8 years ago

On 08/29/2016 04:47 PM, Marcel Ramos wrote:

The Windows machine has an error:

|Error in loadNamespace(i, c(lib.loc, .libPaths()), versionCheck = vI[[i]]) : there is no package called 'codetools' |

@mtmorgan https://github.com/mtmorgan Do you know if the |codetools| error is an SPB quirk?

actually, only a 'WARNING' in terms of R CMD check, even though the WARNING is triggered by an error. And yes, the error involves how the Windows builder was configured -- it may be fixed now.

Martin

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/24#issuecomment-243250830, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHPGKMpBBZF0FY8LgceyikEFTSD3s5hks5qk0VigaJpZM4I3zyE.

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-issue-bot commented 8 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the following build report for more details:

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

rvalieris commented 8 years ago

Hello Marcel,

Thank you for your help during the building of our package 'signeR' on Bioconductor server. I am glad to tell you that the manuscript describing it has been accepted as an original paper for publication in Bioinformatics journal. Last week I received a message that our package was built without errors or warnings on all platforms. Is there any pending issue that could be delaying our package's review? I would like to ask you if there is a permanent link to Bioconductor that we could include in our main article, or if you can speed up the review process so that this link will be available within a few days. The manuscript's copy-editing and formatting is being carried out by the journal office and soon we will be asked to set all details of the final issue version and we would like to make the link available by this time.

Best regards on behalf of the signeR authors.

LiNk-NY commented 8 years ago

Hi @rvalieris, We have reviewed your package and it is ready to be uploaded to SVN. You will soon be provided with SVN credentials.

Thank you for submitting to Bioconductor!

Regards, Marcel

LiNk-NY commented 8 years ago

I have contacted @mtmorgan with regards to getting your package approved quickly or providing a foreseeable URL for the package. I will get back to you on that soon.

LiNk-NY commented 8 years ago

Hi @rvalieris, You can use the following link to your package:

http://bioconductor.org/packages/signeR

Regards, Marcel

Bioconductor / Contributions

signeR #24

dontrun

code style issues

library calls

SignExp class

plots issue