Closed wzthu closed 6 years ago
Hi @wzthu
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: ATACpipe
Type: Package
Title: ATAC-seq Data Quantifying and Annotating Workflow
Version: 0.99.8
Date: 2017-06-25
Author: Zheng Wei, Wei Zhang
Maintainer: Zheng Wei <wzweizheng@qq.com>
Description: This package provides a framework and
complete preset pipeline for the quantification
and analysis of ATAC-seq and DNase-seq Reads.
It covers a complete workflow
starting from raw sequence reads,
over creation of alignments
and quality control report, to the quantification of genomic
regions of interest. The package is managed by dataflow graph,
and users can also build their own pipeline easily and flexibly.
Depends: R (>= 3.4.1), QuasR, Rsamtools, GenomicRanges
License: GPL (>= 3)
Encoding: UTF-8
LazyData: true
Imports:
Rcpp (>= 0.12.11),
R6,
knitr,
Rbowtie2,
rtracklayer,
ggplot2,
Biostrings,
ChIPseeker,
clusterProfiler,
igraph,
rJava,
DiagrammeR,
magrittr,
digest,
BSgenome,
AnnotationDbi,
GenomicFeatures,
R.utils,
GenomeInfoDb,
BiocGenerics,
S4Vectors,
IRanges,
rmarkdown,
tools
Suggests:
BSgenome.Hsapiens.UCSC.hg19,
TxDb.Hsapiens.UCSC.hg19.knownGene,
org.Hs.eg.db
LinkingTo: Rcpp
SystemRequirements: C++11
biocViews: Sequencing, DNASeq, QualityControl, Alignment, Preprocessing, Coverage, ATACSeq, DNaseSeq
VignetteBuilder: knitr
Archs: x64
RoxygenNote: 6.0.1
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.
Received a valid push; starting a build. Commits are:
03a4299 retry version bump
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/ATACpipe_buildreport_20171011104640.html
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, 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/ATACpipe_buildreport_20171011105706.html
Received a valid push; starting a build. Commits are:
f87aa6f retry for timeout
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/ATACpipe_buildreport_20171011115640.html
Hi @mtmorgan,
I renamed ATACFlow #505 as this package(ATACpipe) following @hpages 's suggestion.
Best, Zheng
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/ATACpipe_buildreport_20171015085603.html
Hi Zheng,
Thanks for resubmitting.
Qian Liu is helping me with this review and will provide feedback here very soon. Thanks!
H.
Hi Zheng,
This is a joint review of your package by @hpages and myself. Your package generally looks good, might need some further refinement in the vignette and some of the R code documentation. Please address these issues and reply here when you are ready for a 2nd review. Thanks.
@seealso
within the package documentation to link to main functions of the package.eval=FALSE
. Is it because the time limit for compiling or any other concern? Please check if all eval=FALSE
code chunks are runnable (witho\
ut potential coding error), and if not very time-consuming, make some to be eval=TRUE
.Best, Qian
R6 classes? Why?
Hi @Liubuntu @lawremi ,
Thank you for your suggestions. We will revise the package ASAP.
@Liubuntu , I have an question: You mention "add documentation for S4/R6 classes and methods" and "All man pages need running examples". But all of the classes will not expose to users. They are just used in package. Do we need to add documentation for these classes and their methods? The unexported classes may not be run in examples. Thank you!
The reason for many classes definition and using R6 classes: The integrated elements in ATAC-seq data processing workflow are very multifarious so we need to build a framework that are more easy for us to maintain and extend new elements in the future.
The framework is described below.
R6 is an OOP framework for R. More OOP features are included compared to S3/S4/RC. For example, it supports setting private and public members for classes respectively. Therefore, R6 may provide better classes encapsulation and interfaces realizing. Besides, R6 based on S3, which may be more efficient. That is why we use R6 to implement our package.
Best, Zheng
It is completely untrue that R6 offers more features compared to S4. S4 offers typed fields, multiple dispatch, automatic validation, etc, but most importantly, it's objects can be values or references. R6 is purely reference-based, which is rarely the right thing for an R API. The private/public members is the only thing, and it's debateable whether that is even useful. As every programmer knows, one needs to be careful about public fields. But more importantly, providing two different syntaxes in an API (function call and field access) seems like an unnecessary complication to me. Bioconductor is S4-based, let's keep things in the same framework if at all possible.
Hi Zheng,
You're right that you don't need to document those non-exported methods or classes. Those are just general guidelines for documentation, and you do need to add runnable examples for each exported methods, though you could mark them as \dontrun
if it has prolonged running time when doing the R CMD check
. Also please consider Michael's suggestion of unifying your classes using S4.
Best, Qian
Hi @Liubuntu @lawremi ,
Thank you very much for your reply. Your suggestions are reasonable and we will rewrite the package with S4 classes.
Best, Zheng
Received a valid push; starting a build. Commits are:
cf9801d adding a flowchart and function overview a8ff53c add license file in DESCRIPTION 212f79a change to S4 example a8ea74b atacrenamer to S4 47af8e6 Merge pull request #1 from wzthu/rev atacrenamer ... 299506c fix S4 renamer bfa3c79 Merge branch 'rev' of https://github.com/A1dnoah/A... 29365c1 update des 4216b3f update namespace c2b7fdf update to S4 53f501b Merge pull request #60 from A1dnoah/rev Rev updat... 250afbb samtobed s4 7931456 Merge pull request #61 from A1dnoah/rev samtobed ... 6438bff S4 025ee82 RemoveAdapter to S4 3e61f9c Merge branch 'rev' of https://github.com/wzthu/wzz... c42f8ed bowtie2mapping to S4 38fe442 update S4 0de5283 Merge pull request #62 from A1dnoah/rev S4 918d670 S4 c3da20d Merge pull request #63 from A1dnoah/rev S4 faafb7c Merge pull request #1 from wzthu/rev Rev merge cd78000 fix bug and add S4 FregLenDist 4239b16 S4 1651f16 Merge branch 'rev' of https://github.com/A1dnoah/A... a04483d S4 99bd1da S4 facec0b Merge pull request #65 from A1dnoah/rev Rev f932778 Merge pull request #2 from wzthu/rev Rev dbcd9a3 Merge pull request #66 from A1dnoah/rev Merge pul... e588269 ConfVal to S4, fix getReport f5b2b2a Merge pull request #3 from wzthu/rev Rev 781affe update ATACProc S4 method 5064225 fix bug in FRiPQC 7af2173 update example data 1f8716d remove print 76397e4 fix signature error 0b7d4ac update footprint picture 6c204ac add more explainations 3c3ee38 atacpipe to S4 & fix bug 7114adc Merge pull request #67 from A1dnoah/rev Rev updat... ccc034b Merge pull request #4 from wzthu/rev Rev update a487f46 fix bug 90e9671 Merge branch 'rev' of https://github.com/wzthu/wzz... 4d679cb update vig 2fb9038 Merge pull request #68 from A1dnoah/rev Rev updat... f2aa2d2 Merge pull request #5 from wzthu/rev Rev cacf663 delete R6 cc6fa9b add some text 547b459 Merge pull request #69 from A1dnoah/rev Rev delet... 872aaf2 add parameter 5978623 Merge pull request #70 from A1dnoah/rev add param... 91d9b60 update atacPipe to S4 and fix related bugs 5f5a592 Merge branch 'rev' of https://github.com/wzthu/wzz... 955f069 Merge pull request #6 from wzthu/rev Rev merge ed9e59f fix bugs and add extdata 5313885 update explainations 963f93c Merge pull request #71 from A1dnoah/rev Rev updat... f56b365 add ATACProc S4 documents f0dee74 Merge pull request #72 from A1dnoah/rev add ATACP... 4004c22 Merge pull request #7 from wzthu/rev Rev merge 2f5fb54 add documnet in ConfigVal and Methods 3b46a56 Merge branch 'rev' of https://github.com/wzthu/wzz... 3d73991 update atacpipe2 5a92ceb check spelling cdf4c7f modufy 85638ec Merge pull request #73 from A1dnoah/rev Rev c2972fd Merge pull request #8 from wzthu/rev Rev 6cedea1 add example for ATACProc 328d40d Merge branch 'rev' of https://github.com/wzthu/wzz... a5a9308 add export 95d484c Merge pull request #74 from A1dnoah/rev Rev 2f7a196 Merge pull request #9 from wzthu/rev Rev dee7275 update 22f7145 add ... 8ef3335 Revert "add ..." This reverts commit 22f71457f143... 114efc8 Merge pull request #75 from A1dnoah/rev Rev 07b855d Merge branch 'rev' of https://github.com/wzthu/wzz... 97f92b7 add names and correct link ATACProc 563efa8 add class 4729899 Merge pull request #76 from A1dnoah/rev add class ecfba25 Merge pull request #10 from wzthu/rev Rev 6fac618 add atacproc-classs 8eeafe2 Merge pull request #77 from A1dnoah/rev Merge pul... d128ce8 remove R6 07197b1 Merge pull request #79 from A1dnoah/rev remove R6 59f41ca update number 227b75d update a0a80fb Merge pull request #80 from A1dnoah/rev Rev updat... 02cffd7 Merge pull request #81 from wzthu/rev version 0.9...
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/ATACpipe_buildreport_20171022124228.html
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/ATACpipe_buildreport_20171022132745.html
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, 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/ATACpipe_buildreport_20171022230610.html
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/ATACpipe_buildreport_20171023001423.html
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/ATACpipe_buildreport_20171023025817.html
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/ATACpipe_buildreport_20171023040835.html
Hi @Liubuntu ,
We rewrites all of the R6 classes with S4 classes and revised the package following your suggestions. The only ERROR still comes from importing QuasR, which do not support win32 platform, mentioned before. We are ready for next review now. Would you please to have a look at our package? Thanks!
These are the changes we made.
Best, Zheng
HI Zheng,
Thank you for your quick response for these changes. The summary looks good. I will take another look at your package and get back with you very soon.
Best, Qian
Hi Zheng,
It's very impressive that you could make major changes within several days. After a second review from @hpages and myself, there are still some minor issues you need to address. If all done within 2-3 days, we may still consider to include it in the Bioc 3.6 release. Thank you for your contribution to Bioconductor.
ATACpipe.R
is an empty file with 0 lines (0 sloc), 0 Bytes. chr20_1.1.fq.bz2
, chr20_1.fq.bz2
, chr20_sample_peak.bed.bz2
)Consider adding
importFrom("grDevices", "dev.off", "pdf")
importFrom("graphics", "abline", "axis", "plot")
importFrom("stats", "fft", "runif")
importFrom("utils", "download.file", "read.table", "write.table")
Please also import masks, DNAStringSet, injectHardMask from Biostrings. Check your no visible global function definition
problems for potential non-imported functions.
Other 'no visible global function' NOTEs seem like errors in your code e.g.
initialize,SamToBam: no visible binding for global variable bamInput
in SamToBam.R
file where the initialize,SamToBam method is implemented, it makes reference to a bamInput
variable that doesn't seem to be defined anywhere.
Please investigate all these NOTEs and address them.
Best, Qian
Hi @Liubuntu , Thank you for your quickly reply and your suggestions ! We will revise the package ASAP. Our package is "An Easy-to-use Systematic Pipeline for ATACseq data analysis" . So we want to change our package name as sysATAC or esATAC, can we? Thanks !
Hi Zheng,
Please do the following for changing your package name:
Update the following URL to point to the GitHub repository of
the package you wish to submit to Bioconductor
**Repository: https://github.com/wzthu/ATACpipe**
do a version bump
it will fail to build, and requires some manual intervention to update our database of issues. Let me know when you've updated your repo, and someone in Bioconductor core team will update the data base and trigger a build manually.
Qian
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/ATACpipe_buildreport_20171027121906.html
Hi Qian @Liubuntu ,
I have updated the package repo for changing package name. We are ready now. Would you please help us update the database of issue? Thank you for your help !
Best, Zheng
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/esATAC_buildreport_20171027123851.html
HI Zheng,
The name change of your package is successful and the database is updated. Please address the other issues we returned. Thanks.
Best, Qian
Received a valid push; starting a build. Commits are:
d439429 add import from bowtie2 bowtie2_build b301f3b change report format b2e287e add JASPAR human motif PWM ec8721d update affab29 Merge pull request #82 from A1dnoah/devel Devel 4aa2933 delete e79af60 add bb759c0 update report 44bb2dc update Rd file fe45c72 fix no cut site stop error 285902a add and remove edge 2aeaeaa update pipeline d407df2 print motif name in screen 6ae614a new function to case ctrl 05c47af update function b2229ec update d45efe8 Merge pull request #83 from A1dnoah/devel Devel u... 0fffd86 using all human motif in JASPAR2016 5f26113 change folder where the output saved 857e1c2 modify some spellings cc027a5 Merge pull request #84 from A1dnoah/devel Devel u... b20e9b8 kable add align = "l" cb68319 add new parameter PWM 0a4e835 Merge pull request #86 from A1dnoah/devel Devel u... b0713a6 Merge pull request #87 from wzthu/master merge ma... 5ab404c add more unit test and fix bug 8c28109 fix bug for changing package name 4908910 add packages 674b9c9 add parameter ... 690cebf add additional footprint plot 959d6a6 update man 3648d9d add unit test and fix bugs c8b729c Merge pull request #88 from A1dnoah/devel Devel u... 07d5095 Merge branch 'rev' into devel ffcc407 update peakcomp 2906c3e Merge pull request #90 from A1dnoah/devel update ... 8def2fe fix note 7eda6e7 Merge pull request #91 from A1dnoah/devel fix not... 2b40954 fix note 7c330c8 Merge branch 'devel' of https://github.com/wzthu/w... ec3a202 Merge pull request #92 from wzthu/devel Devel
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, 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/esATAC_buildreport_20171028093321.html
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, 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/esATAC_buildreport_20171028102154.html
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/esATAC_buildreport_20171028115353.html
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/esATAC_buildreport_20171028122359.html
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/esATAC_buildreport_20171028125638.html
Hi @Liubuntu @hpages , Thank you again for your suggestion and your help. We have revised our package following your suggestions. Would you please have a look at our package whether it is ready to be accepted ? Thank you very much!
These are the changes we made:
Best, Zheng
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]'
[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:
For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.