jl354 commented 7 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/jl354/bcSeq

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 Git (optionally via GitHub).

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 @jl354

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: bcSeq
Type: Package
Title: Fast Sequence Alignment for High-throughput shRNA and CRISPR Screens
Version: 0.99.0
Date: 2017-09-20
Author@R: c(person("Jiaxing","Lin", role = c("aut", "cre"), 
  email = "jiaxing.lin@duke.edu"), 
  person("Jeremy", "Gresham", role = "aut"),
  person("Jichun", "Xie", role = "aut"),
  person("Kouros", "Owzar", role = "aut"),
  person("Tongrong", "Wang", role = "ctb"),
  person("So Young", "Kim", role = "ctb"),
  person("James", "Alvarez", role = "ctb"),
  person("Jeffrey", "Damrauer", role = "ctb"),
  person("Scott", "Floyd", role = "ctb"),
  person("Joshua", "Granek", role = "ctb"),
  person("Andrew", "Allen", role = "ctb"),
  person("Cliburn", "Chan", role = "ctb"))          
Author: Jiaxing Lin [aut, cre],
        Jeremy Gresham [aut],
        Jichun Xie [aut],
        Kouros Owzar [aut],
        Tongrong Wang [ctb],
        So Young Kim [ctb], 
        James Alvarez [ctb], 
        Jeffrey S. Damrauer [ctb],
        Scott Floyd [ctb], 
        Joshua Granek [ctb], 
        Andrew Allen [ctb],
        Cliburn Chan [ctb]
Maintainer: Jiaxing Lin <jiaxing.lin@duke.edu>
Description: This Rcpp-based package implements a highly efficient
    data structure and algorithm for performing alignment of short
    reads from CRISPR or shRNA screens to reference barcodes libraries
    based on user specified mismatch or INDEL tolerance. Matching
    qualities are evaluated based on Phred scores.  A Bayes'
    classifier is employed to predict the originating barcode of a
    read. The package supports provision of user-defined probability
    models for evaluating matching qualities. The package also
    supports multi-threading.
License: GPL (>= 2)
biocViews: Alignment, CRISPR, Sequencing, SequenceMatching, MultipleSequenceAlignment, Software
URL: https://bitbucket.org/impactp01/bcSeq
Imports: Rcpp (>= 0.12.12), Matrix
LinkingTo: Rcpp, Matrix
Suggests: knitr
VignetteBuilder: knitr
BuildVignettes: yes
NeedsCompilation: yes

lawremi commented 7 years ago

Needs to integrate with Bioc data structures. Presumably sequences could be passed from as DNAStrings and results returned in GAlignments or GRanges.

Also, if this is a novel algorithm written in C++, why not make it a separate C++ library, with this package as the R interface to it?

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: "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/bcSeq_buildreport_20170925160255.html

jl354 commented 7 years ago

Hi lawremi @lawremi

Will try to add returns in GAlignments, however, the package meant to deal with the Phred score. I am not quite sure if the DNAstring can be implemented, do you have some examples.
It is incorporating some nice C++ features for both DNA sequence and phred score, not a quite independent C++ library. Also, it would be handy to add in with Rcpp.
The windows build of our package has some error that is similar to the post below: https://github.com/Bioconductor/packagebuilder/issues/2
I have tried my own window system for building and CRAN windows builder. Both worked fine. Can you suggest what we can do with this? Bests Jiaxing

lawremi commented 7 years ago

Should be possible to incorporate any additional scores into GAlignments in the metadata columns. I'm not sure what kind of examples you need for DNAString. I think the user would want to pass sequences in a DNAString(Set) to your aligner.

bioc-issue-bot commented 7 years ago

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

6b2e4ef version number bumpped

jl354 commented 7 years ago

@lawremi Hi I had update the package per your recommend for add support for the DNAString(Set) as package aligner input.

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: "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/bcSeq_buildreport_20170929225059.html

bioc-issue-bot commented 7 years ago

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

1ac3bcc fix unit test error

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

bioc-issue-bot commented 7 years ago

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

b6e3a2f version to 0.99.3

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

bioc-issue-bot commented 7 years ago

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

dbf02dd fixed cerr

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

bioc-issue-bot commented 7 years ago

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

7edfd8b fix window warnings

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

LiNk-NY commented 7 years ago

Hi JianXing, @jl354

Thank you for your submission to Bioconductor. Please find the review below.

If you have any questions, feel free to contact me.

Best regards, Marcel

bcSeq #495

bcSeq is a fast aligner for shRNA and CRISPR.

You've updated the package to support DNAString(set)

DESCRIPTION

Please include a Depends field
Import additional packages based on @lawremi 's review
Include a BugReport field

NAMESPACE

Looks good

R

Please remove "Error!" word from error message. stop already labels it as an error.
It looks like you're overwriting tMatSeq[1] with tMatSeq[1] <- tMat[1, 1], it doesn't seem necessary
Avoid 1:nrow(x). Use seq_len(nrow(x)) instead for robustness
Use is.character and related functions to check for S3 class membership

vignettes

I wasn't able to pull up the vignette after installation. I used:

# R CMD INSTALL bcSeq_* && R
library(bcSeq)
browseVignettes("bcSeq")
vignette(package = "bcSeq")

Other than that, the code runs without issues

bioc-issue-bot commented 7 years ago

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

7e9608a modification for review

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

jl354 commented 7 years ago

Hi Marcel, @LiNk-NY

Thanks you so much for the suggestive comments on the package. We have made modification according to your comments as below:

*DESCRIPTION (1) added Depends filed (2) imported DNAStrings (3) included a BugReport filed

*R (1) remove "Error!" word (2) Explain: in the beginning we set up a default value for tMatSeq[1] and we check if the user supply a vectir fir tMatSeq, if it is true, we put the vector to tMatSeq and make tMatseq a vector. Thus, we want an over write. However, better solution are welcomed. (3) changed 1:nrow(x) to seq_len(nrow(x)) for robustness (4) Used is.character() for checking S3 class.

vignettes I try the following code and it can pull up the vignette: R CMD INSTALL bcSeq_ && R library(bcSeq) vignette("bcSeq", package = "bcSeq")

Again, thank you so much for taking time for review our package. Also, the modified code pass through the build check. Please let us know if you have other questions.

Bests

Jiaxing

LiNk-NY commented 6 years ago

Hi JiaXing, @jl354 Please use is.character(sampleFile) and is(sampleFile, "DNAStringSet") respectively for S3 and S4 class tests in your code. It's pretty close.

Thanks, Marcel

bioc-issue-bot commented 6 years ago

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

1dbfe62 use is.class() to check S3 and S4 class instead of...

jl354 commented 6 years ago

Hi Marcel @LiNk-NY

Thanks for the careful review, I have update the S3 and S4 test methods according to your suggestion.

Bests

Jiaxing

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

jl354 commented 6 years ago

Dear Marcel @LiNk-NY

I have pushed your proposed changes from 10/26. The revised package builds successfully.

Could you please advise if additional revisions are needed to get our package approved?

Sincerely, Jiaxing @jl354

LiNk-NY commented 6 years ago

Hi JiaXing, @jl354 Sorry for the delay. Package reviews were put on the back burner for a bit. I will have a quick look at the package and likely approve of it shortly. Best regards, Marcel

bioc-issue-bot commented 6 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!

jl354 commented 6 years ago

Dear Marcel @LiNk-NY

May I please ask when our package is expected to be added to the Bioconductor git repository?

Happy Thanksgiving.

Sincerely, Jiaxing @jl354

mtmorgan commented 6 years ago

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

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/jl354.keys is not empty), then no further steps are required. Otherwise, do the following:

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(\"bcSeq\"). The package 'landing page' will be created at

https://bioconductor.org/packages/bcSeq

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.

Bioconductor / Contributions

bcSeq #495

bcSeq #495

DESCRIPTION

NAMESPACE

R

vignettes