search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

DMCHMM #430

Closed shokoohi closed 7 years ago

shokoohi commented 7 years ago

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

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

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

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

bioc-issue-bot commented 7 years ago

Hi @shokoohi

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: DMCHMM
Type: Package
Title: Differentially Methylated CpG using Hidden Markov Model
Version: 0.99.0
Author: Frhad Shokoohi
Maintainer: Frhad Shokoohi <shokoohi@icloud.com>
Description: DMCHMM is a novel profiling tool for identifying 
    differentially methylated CpG sites using Hidden Markov Model in bisulfite
    sequencing data.
Depends: R (>= 3.4.0),
    SummarizedExperiment,
    methods,
    S4Vectors,
    doSNOW,
    GenomicRanges,
    IRanges,
    fdrtool
Imports: utils,
    stats,
    parallel,
    foreach,
    grDevices,
    rtracklayer,
    multcomp,
    calibrate,
    graphics
Suggests: testthat, 
    RUnit,
    BiocGenerics,
    knitr
VignetteBuilder:
    knitr
biocViews: DifferentialMethylation, Sequencing, HiddenMarkovModel, Coverage
License: GPL (>= 3)
Date: 2017-06-23
Encoding: UTF-8
LazyData: true
BugReports: https://github.com/shokoohi/DMCHMM/issues
RoxygenNote: 6.0.1
NeedsCompilation: no
Packaged: 2017-06-23 00:13:02 UTC; Farhad
bioc-issue-bot commented 7 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 7 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, TIMEOUT, 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/DMCHMM_buildreport_20170727042104.html

shokoohi commented 7 years ago

Hello,

The issue is solved now.

Thank you,

Farhad Shokoohi, Assistant Professor Concordia University

On Jul 27, 2017, at 4:21 AM, bioc-issue-bot <notifications@github.com mailto:notifications@github.com> wrote:

Dear Package contributor,

This is the automated single package builder at bioconductor.org http://bioconductor.org/.

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

On one or more platforms, the build results were: "WARNINGS, TIMEOUT, 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/DMCHMM_buildreport_20170727042104.html http://bioconductor.org/spb_reports/DMCHMM_buildreport_20170727042104.html — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/430#issuecomment-318293422, or mute the thread https://github.com/notifications/unsubscribe-auth/AOWvF62-M6i4Bz5kYGDJX-HqepWS4uAJks5sSEhygaJpZM4Okrg3.

shokoohi commented 7 years ago

Hello,

I haven’t received any news regarding my submitted package “DMCHMM”. The issue is solved and I sent an email long time ago. Would you please update me on stage of reviewing my package?

Thank you, Farhad Shokoohi Assistant Professor, Concordia University

nturaga commented 7 years ago

@shokoohi Please correct the Error's on your package build report . I will review your package soon.

Please bump the version, and also please check the spellings in your DESCRIPTION file.

shokoohi commented 7 years ago

Hello,

I have already subscribed to bio-devel mailing list after I received the report mentioning the following error:

ERROR: Maintainer must subscribe to the bioc-devel mailing list. Subscribe here: https://stat.ethz.c

On Aug 7, 2017, at 3:57 PM, Nitesh Turaga notifications@github.com wrote:

@shokoohi Please correct the Error's on your package build report . I will review your package soon.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

nturaga commented 7 years ago

You have to bump the version in the DESCRIPTION file and commit. Check maintainer name as well.

https://github.com/shokoohi/DMCHMM/blob/master/DESCRIPTION

bioc-issue-bot commented 7 years ago

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

8c0bca3 Version 0.99.1

bioc-issue-bot commented 7 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: "TIMEOUT, 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/DMCHMM_buildreport_20170807190305.html

bioc-issue-bot commented 7 years ago

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

6faafaf Version 0.99.2 Updated examples to run faster.

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170807195439.html

shokoohi commented 7 years ago

Hello,

The warnings are not related to my package. It happens because of other packages that I cannot solve. If you have any solution let me know to fix it.

Thanks,

On Aug 7, 2017, at 7:54 PM, bioc-issue-bot notifications@github.com wrote:

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/DMCHMM_buildreport_20170807195439.html http://bioconductor.org/spb_reports/DMCHMM_buildreport_20170807195439.html — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/430#issuecomment-320811655, or mute the thread https://github.com/notifications/unsubscribe-auth/AOWvF2QBOuEGJEi95iuzVm7fd8Lcncoeks5sV6PAgaJpZM4Okrg3.

bioc-issue-bot commented 7 years ago

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

8ba1089 0.99.3 renamed BSData and BSDMCs to get rid of som...

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808104149.html

shokoohi commented 7 years ago

I have no idea why your build is not working. Everything is fine on my computer.

bioc-issue-bot commented 7 years ago

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

ae02013 0.99.4

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808110140.html

bioc-issue-bot commented 7 years ago

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

c280495 0.99.5

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808112541.html

bioc-issue-bot commented 7 years ago

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

6c8bffe Merge remote-tracking branch 'origin/master' ec0fa7c 0.99.6

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808125300.html

bioc-issue-bot commented 7 years ago

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

d886a37 0.99.7

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808133002.html

bioc-issue-bot commented 7 years ago

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

f391dda 0.99.8

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808140427.html

bioc-issue-bot commented 7 years ago

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

888e354 0.99.9

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170808143521.html

dvantwisk commented 7 years ago

Hello,

@nturaga has been busy this week completing a large project, so I will now be reviewing this package. We'll try to have a review in to you by mid next week.

dvantwisk commented 7 years ago

We've reviewed your package! Please address the following issues and message us back when you are finished:

General:

DESCRIPTION:

NAMESPACE:

R/AllClasses.R

R/AllGenerics.R

R/DMCHMM-package.R

R/cBSDMCs-method.R

R/cBSData-method.R

R/combine-method.R

R/data.R

R/findDMCs.R

R/manhattanDMCs-method.R

R/methHMEM-method.R

R/methHMMCMC-method.R

R/methLevel-method.R

R/methReads-method.R

R/methStates-method.R

R/qqDMCs-method.R

R/readBismark-method.R

R/totalReads-method.R

R/writeBED-method.R

R/zzz.R

tests

vignettes

bioc-issue-bot commented 7 years ago

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

0d3c608 Version: 0.99.11

bioc-issue-bot commented 7 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: "TIMEOUT". 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/DMCHMM_buildreport_20170822203115.html

bioc-issue-bot commented 7 years ago

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

276dec5 Version: 0.99.12

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170823104614.html

shokoohi commented 7 years ago

Hello, Most of the comments that I received are implemented. Here are the things that are implemented and those that are not implemented.

General:

[done] Add DMCHMM.Rproj to .gitignore. [done] Indentation in R files varies greatly and in R files that contain little to no identation, the code is very difficult to read. In these files, use a formatting tool like the formatR package to improve readability. Keep in mind that using this tool will change the line numbers which we are using to locate the issues with your code. For this reason, use the formatR package after you correct the issues below so you know where to locate the problems. DESCRIPTION:

[done] L20: Use the package BiocParallel instead of parallel. [done] LICENSE: Pick specific GPL version. NAMESPACE:

Looks great! R/AllClasses.R

[not done] Consider placing parameters inside individual roxygen2 headers instead of placing them all in one block. R/AllGenerics.R

R/DMCHMM-package.R

R/cBSDMCs-method.R

[Couldn't figure out what the comment meant] L1-18: May be better to create private cBSDMCs constructor from cBSDMCs's setClass() function, and use a single private constructor to add custom input to the private function. This code block doesn't need to call new(). .cBSDMCs <- setClass("cBSDMCs's", ....) R/cBSData-method.R

Same as above. R/combine-method.R

R/data.R

[done] Provide a better description of what the data file contains. For example, explain more about what it is and how to data is organized. R/findDMCs.R

[done] L13: Improper use of paste() in stop() function. What is there now will create a character vector with the length of names(colData(object)) elements that repeat the initial statement. Replace with the following the the desired effect: `stop("The variables ....", paste(names(colData(object)), collpase=", "), ".") [REQUIRED] L52-59: The sapply()'s here are used multiple times and there is no need to run this command multiple time. Declare a variable once and reuse it. [done] L171-173: Replace inefficient copy pattern with: pval <- vapply(totres3, function(elt) elt$p.val, numeric(1)) [REQUIRED] L183-187: Replace inefficient copy and append pattern with apply's.

[done] L243-248: Similar issue as L171-173.

R/manhattanDMCs-method.R

[done] L77-81: Code can be vectorized as follows: udCHR <- unique(d$CHR) i <- seq_along(udCHR) idx <- match(d$CHR, udCHR) d[idx, ]$index <- i[idx] L104-110: [done] Similar issue to L77-81. Try to vectorize it. [REQUIRED] L206-211: Replace with: topSNPs <- lapply(unique(topHits$CHR), function(i) { chrSNPs <- topHits[topHits$CHR == i, ] }) topSNPs <- do.call(rbind, topSNPs)
R/methHMEM-method.R

[done] L41: Vectorize code: Pm <- t(exp(Pm)/rowSums(Pm)) [done] L107-108: new.Pm[j, ] <- rowSums(Wijk.Estep[, j, ])/sum(Wijk.Estep[, j,]) [done] L165-167: Replace with vectorized code: I am assuming that all vectors have the same length, here: mlike <- sum(nw dbinom(Yv, nv, X.estimate, log=TRUE)) If they don't have the same length vals <- seq_len(nPos) mlike <- sum(nw[vals] dbinom(Yv[vals], nv[vals], X.estimate[vals], log=TRUE)) [done] L304-311: Use applys to remove inefficient copy and append pattern. R/methHMMCMC-method.R

[done ] L87-90: Vectorize like mentioned above in L165-167. [not done since the code is recursive in some lines] L117-145: All code can be vectorized here. [didn't understand] L159-165: use aggregate() here. [done] L306-318: Vectorize code to avoid expensive copy like mentioned above in L296-303. R/methLevel-method.R

R/methReads-method.R

R/methStates-method.R

R/qqDMCs-method.R

R/readBismark-method.R

[done] L9: Use seq_along() here instead of 1:length(files) to avoid unexpected case where 1:0 results in a vector of 1 0. [done] L42: Use seq_long() as opposed to seq(along=....). R/totalReads-method.R

R/writeBED-method.R

[not done] Writing to a BED file appears to have already been implemented in Bioconductor in rtrackalyers in the function asBED(). It would be better to reuse this function by writing a shorted adapter wrapper that will allow your object to be written using the function instead of writing new code reimplement it just for your objects. R/zzz.R

[done] utils::packageDescription parsed multiple times. Pull it out of a variable instead of reading it multiple times. [done] L6: no paste needed here. tests

[done] Consider adding tests using the testthat package. vignettes

shokoohi commented 7 years ago

Hello The package is ready for another review. Would you please let me know the status? Thank you.

dvantwisk commented 7 years ago

Here is our second review, please address these issues and comment back when you are finished.

GENERAL

DESCRIPTION

R/findDMCs.R

shokoohi commented 7 years ago

Hello The multicoreWorkers() function in BioParallel detecets less core than detectCores() in parallel package. For instance, on my Mac, the total cores are 8, but the multicoreWorkers() detects 6. How can I fix this problem if I want to remove parallel package from my package? Best,

bioc-issue-bot commented 7 years ago

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

bc6dbb2 Version 0.99.13 Following comments are applied: ...

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170829191843.html

shokoohi commented 7 years ago

Hello,

I don't understand the error. How can I fix it?

Best

shokoohi commented 7 years ago

Hello Following comments are applied:

GENERAL

[Done] delete commented out code blocks if they are no longer going to be used. DESCRIPTION

[Done] remove parallel from Depends if BiocParallel is being used. R/findDMCs.R

[Done] L290: remove all variables but qval from declaration. [Done] L308-324: Inefficient copy and append pattern with namesPairDMC and namesPairDIR variables. Replace with applies or vectorized code, if able.

Thank you

mtmorgan commented 7 years ago

multicoreWorkers() is friendly, returning by design fewer cores than available. See ?multicoreWorkers

     'multicoreWorkers()'. On windows, the number of multicore workers
     is always 1. Otherwise, the default is the maximum of 1 and
     'parallel::detectCores() - 2'; machines with 3 or fewer cores are
     assigned a single worker. The option 'mc.cores' can be used to
     specify an arbitrary number of workers, e.g.,
     'options(mc.cores=4L)'; the _Bioconductor_ build system enforces a
     maximum of 4 workers.

This is basically to be nice, and not consume all resources on your individual computer or on a server. The '*Param' objects all have as first argument the number of workers, so this can be easily over-ridden.

mtmorgan commented 7 years ago

Windows doesn't support 'fork' processes, so in effect has just 1 multicore worker; your vignette invokes with 2 workers, which is greater than 1.

For this reason, on Windows the default parallel environment is SnowParam(), and BiocParallel arranges for this default automatically -- check out registered() on different platforms; for Linux the highest priority back-end is MulticoreParam(), but for Windows it's SnowParam()

As a package developer you could re-create this special handling, but from the vignette the recommend practice is to simply not provide a BPPARAM argument to BiocParallel -- it'll pick up the param that has been registered() by default, or by the user (who might have important reasons for choosing, e.g., not to use all 128 cores on their University cluster).

bioc-issue-bot commented 7 years ago

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

e6a9aa3 Version 0.99.14

bioc-issue-bot commented 7 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/DMCHMM_buildreport_20170831132025.html

shokoohi commented 7 years ago

The package is ready for further evaluation. Thanks

dvantwisk commented 7 years ago

Sorry for the delay! Things look fine now!

bioc-issue-bot commented 7 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be sent to the maintainer email address in the next several days.

Thank you for contributing to Bioconductor!

mtmorgan commented 7 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

To use this repository, we need an 'ssh' key to associate with your github user name. Please

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using biocLite("YOUR_PACKAGE_NAME"). The package 'landing page' will be created at

https://bioconductor.org/packages/YOUR_PACKAGE_NAME

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.