Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

snapcount package submission #1304

Closed ch4rr0 closed 4 years ago

ch4rr0 commented 4 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 4 years ago

Hi @ch4rr0

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: snapcount
Type: Package
Title: R/Bioconductor Package for interfacing with Snaptron for rapid querying of expression counts
Version: 0.99.0
Author: Rone Charles
Maintainer: Rone Charles <rcharle8@jh.edu>
Description: snapcount is a client interface to the Snaptron webservices
    which support querying by gene name or genomic region.
    Results include raw expression counts derived from alignment of RNA-seq samples
    and/or various summarized measures of expression across one or more regions/genes
    per-sample (e.g. percent spliced in).
Depends:
    R (>= 3.2.0)
Imports:
    R6,
    httr,
    rlang,
    purrr,
    jsonlite,
    assertthat,
    data.table,
    Matrix,
    magrittr,
    stringr,
    stats,
    IRanges,
    GenomicRanges,
    SummarizedExperiment
Suggests:
    BiocManager,
    bit64,
    covr,
    knitcitations,
    knitr (>= 1.6),
    JunctionSeq,
    devtools,
    BiocStyle (>= 2.5.19),
    rmarkdown (>= 0.9.5),
    testthat (>= 2.1.0)
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1
Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "collate"))
URL: https://github.com/langmead-lab/snapcount
BugReports: https://github.com/langmead-lab/snapcount/issues
biocViews: Coverage, GeneExpression, RNASeq, Sequencing, Software, DataImport
VignetteBuilder: knitr
ByteCompile: true

Add SSH keys to your GitHub account. SSH keys will are used to control access to accepted Bioconductor packages. See these instructions to add SSH keys to your GitHub account.

bioc-issue-bot commented 4 years ago

A reviewer has been assigned to your package. Learn what to expect during the review process.

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 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

f9e5490 Update minimum R version to 3.5 b637fca Use is instead of class for checking object inheri... ef77aac Serialize objects using RDS version 2

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

1e5d354 Remove RStudio project folder cfc17b4 Update base R version to 3.6.0

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

f4bd00f Update travis url

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

ce6b817 Remove file in License field and import "is" funct...

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

e58a506 Add methods to Imports list

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

f4d09f2 Update R base version to 4.0 to get suppress warni...

bioc-issue-bot commented 4 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 build report for more details.

ch4rr0 commented 4 years ago

@hpages -- The project now compiles without warning or error and would love to continue the review process.

hpages commented 4 years ago

Hi @ch4rr0 ,

Happy New Year and sorry for the delay.

Looks like the snapcount package is using reference classes. Any reason for that? Bioconductor strongly recommends the use of traditional S4 classes, unless you have a good reason for using something else.

In any case, Bioconductor users are not necessarily familiar with the sb$query_jx() syntax and should not be required to learn it in order to use your package. Please make sure that your objects support the standard query_jx(sb) syntax, even if they are reference objects (reference objects also support it), and stick to this syntax in all your examples and vignette.

Thanks! H.

ch4rr0 commented 4 years ago

I am working on this.

hpages commented 4 years ago

Any progress on this? Thanks

ch4rr0 commented 4 years ago

Still working on the changes. Some additional suggestion were brought during an internal review, so working on addressing those as well.

ch4rr0 commented 4 years ago

I will be resubmitting the package tomorrow. Apologies for the delay.

hpages commented 4 years ago

No problem. Thanks for the update.

ch4rr0 commented 4 years ago

FYI -- I need to fix some test failures caused by recent changes, so will be delayed by a day.

bioc-issue-bot commented 4 years ago

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

084c3cd Bump version b0220d9 Update version to 4.0.0

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

1036202 Replace is with class and bump version

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

186ab3e Fix bioductor warnings and bump version

bioc-issue-bot commented 4 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 build report for more details.

ch4rr0 commented 4 years ago

@hpages -- package is ready for review now.

ch4rr0 commented 4 years ago

@hpages -- do you think we will be ready for the next bioconductor release?

ch4rr0 commented 4 years ago

any updates?

hpages commented 4 years ago

Hi @ch4rr0 ,

Sorry again for the delay.

My major concern remains: the package is still using reference classes. Sorry to ask again but are there any reasons for that? See my comment from Jan 9 above in this issue, and also, if the use of reference classes is really necessary, my recommendation to avoid exposing the end user to things like sb$compilation("gtex")$regions("CD99")$query_jx().

Other minor things:

  1. Please address the R CMD check NOTEs about the LICENSE file.

  2. Using URLs like https://bioconductor.org/packages/release/bioc/vignettes/recount/inst/doc/recount-quickstart.html https://bioconductor.org/packages/release/bioc/html/SummarizedExperiment.html is fragile because these URLs are not guaranteed to work in the future. It's much better to use the "canonical URL" for a Bioconductor package (sometimes called the Package Short Url). The canonical URL is of the form: https://bioconductor.org/packages/pkgname. The Bioconductor project is committed to keep these URLs valid for as long as the project will live.

  3. In ?Compilation:

    If the package cannot connect to the internet the variants will default to:

    but there is nothing after "to:"

  4. Too many typos e.g. SnaptonQueryBuilder ("r" missing), RangeSummarizedExperiment ("d" missing), etc... Please re-read carefully your man pages and the vignette.

Thanks, H.

ch4rr0 commented 4 years ago

The reference class makes it easy to implement the Builder pattern used in SnaptronQueryBuilder class. We kept it documented, but also provided wrapper functions for users not familiar with R6. I made an attempt to replace all examples outside of the SnaptonQueryBuilder man page with wrapper functions.

ch4rr0 commented 4 years ago

I am working on fixing the remaining issues.

ch4rr0 commented 4 years ago

FYI -- we are almost done with the changes.

bioc-issue-bot commented 4 years ago

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

289d748 Bump version

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

cfab800 Add missing sample_names param to getJunctionSeq... fcd47d0 Address license issue d20655f Update documentation and bump version

bioc-issue-bot commented 4 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: "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 build report for more details.

ch4rr0 commented 4 years ago

There seems to be an issue with the build bot.

bioc-issue-bot commented 4 years ago

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

f1bfa2d Bump version to retrigger build

bioc-issue-bot commented 4 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: "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 build report for more details.

hpages commented 4 years ago

We're currently looking into this. Thanks!

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

19564fa Fix license and bump version 016ac18 Remove reference to SnaptronQueryBuilder f8b5bac Bump version

bioc-issue-bot commented 4 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 build report for more details.

hpages commented 4 years ago

Thanks for all the improvements to the package. It's in a much better shape. I like that the user can now operate on the SnaptronQueryBuilder objects using the classic R syntax. Note that you still have a few occurrences of "SnaptonQueryBuilder" (the "r" is missing) but that is not a showstopper (you can fix them now or after the package is added to our git server at git.bioconductor.org).

I'm accepting the package.

Cheers, H.

bioc-issue-bot commented 4 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.

Thank you for contributing to Bioconductor!