Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

CAGEfightR #659

Closed MalteThodberg closed 6 years ago

MalteThodberg commented 6 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 6 years ago

Hi @MalteThodberg

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: CAGEfightR
Type: Package
Title: Analysis of Cap Analysis of Gene Expression (CAGE) data using
    Bioconductor
Version: 0.3.1
Author: Malte Thodberg
Maintainer: Malte Thodberg <maltethodberg@gmail.com>
Description: Includes functions for: Finding Tag Cluster (TCs), quantifying
    expression as an Expression Matrix (EM), annotating TCs using transcript models,
    finding Bidirectonal Clusters (BCs).
URL: https://github.com/MalteThodberg/CAGEfightR
BugReports: https://github.com/MalteThodberg/CAGEfightR/issues
Depends:
    R (>= 3.4),
    GenomicRanges (>= 1.30.1),
    rtracklayer (>= 1.38.2),
    SummarizedExperiment (>= 1.8.1)
Imports:
        pryr(>= 0.1.3),
        assertthat(>= 0.2.0),
        methods,
        Matrix(>= 1.2-12),
        Matrix.utils(>= 0.9.6),
        grr,
    BiocGenerics(>= 0.24.0),
    S4Vectors(>= 0.16.0),
    IRanges(>= 2.12.0),
    GenomeInfoDb(>= 1.14.0),
    GenomicFeatures(>= 1.29.11),
    BiocParallel(>= 1.12.0),
    GenomicFiles(>= 1.14.0),
    Gviz(>= 1.22.2)
License: GPL-3 + file LICENSE
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.0.1
Suggests: knitr,
    rmarkdown,
    BiocStyle,
    org.Mm.eg.db,
    TxDb.Mmusculus.UCSC.mm9.knownGene
VignetteBuilder: knitr
biocViews: Software, Transcription, Coverage, GeneExpression, GeneRegulation, PeakDetection, DataImport, DataRepresentation, Transcriptomics, Sequencing, Annotation, GenomeBrowsers, Normalization, Preprocessing, Visualization
bioc-issue-bot commented 6 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 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.

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

bioc-issue-bot commented 6 years ago

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

e9621f5 Slightly changed assignTxType code to not using ap... 82d68f0 Merge branch 'master' of github.com:MalteThodberg/...

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.

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

bioc-issue-bot commented 6 years ago

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

c32531a Replaced direct coercion to RangesList by manual s... 13d8b08 Merge branch 'master' of github.com:MalteThodberg/...

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.

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 6 years ago

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

96d5650 More Biochecks: Registered mail in BOTH support an...

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.

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.

MalteThodberg commented 6 years ago

Dear @nturaga:

The package now passes checks, except the warning for having too low an R-version.

I would greatly prefer to keep the R-version at 3.4, as we are currently several people using the package by installing from GitHub via devtools. I would be super inconvenient to have to force all these users to use the development version of R. When the release date approaches, I will be happy to bump up the R-version.

MalteThodberg commented 6 years ago

Dear @nturaga,

Please do let me know if there's anything I need before the review process to move along. I'm more than happy to implement changes and suggestions.

pkimes commented 6 years ago

Just a quick comment from a fellow R developer (not affiliated with Bioconductor) regarding the R versions.

Why not just create a separate branch that uses R version 3.4 (maybe called R-3.4) and update your master branch to meet the Bioconductor requirements? Collaborators can still install the package using devtools with the ref = argument:

devtools::install_github("MalteThodberg/CAGEfightR", ref = "R-3.4")

Alternatively, if you don't think you'll be updating the R-3.4 version of the package (and therefore don't need a separate branch), you can just use the current commit hash (or create a tag) as the value to ref =:

devtools::install_github("MalteThodberg/CAGEfightR", ref = "96d5650")
nturaga commented 6 years ago

@MalteThodberg I will review your package in the next week.

nturaga commented 6 years ago

Hi @MalteThodberg Please remove the .html and the .R files from your vignettes directory, and then issue a version bump as i'm unable to run the vignette on my end,

> TSSs <- assignTxType(TSSs, txModels=txdb)
Error: seqlengths(object) not identical to seqlengths(txModels)
> enhancers <- assignTxType(enhancers, txModels=txdb)
Error: seqlengths(object) not identical to seqlengths(txModels)
bioc-issue-bot commented 6 years ago

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

9ae29bc Deleted ignored vignette files. 7aa00ce Merge branch 'master' of github.com:MalteThodberg/...

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.

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.

nturaga commented 6 years ago

@MalteThodberg Make sure to check all the NOTE messages you get in the build report and correct as many as possible. Most of them are pretty easy to fix.

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.

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 6 years ago

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

3b20096 Updated version to R devel. 7cd3315 Fixed bug with missing BSgenome 369d6ed Added NEWs 0117898 Fixed lines longer than 80 characters e8d569a bumped version for note fixes

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.

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

bioc-issue-bot commented 6 years ago

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

f61d0e2 R version back to 3.5 a3c790f Merge branch 'master' of github.com:MalteThodberg/...

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

MalteThodberg commented 6 years ago

I've tested the package under Rdevel 3.6 now, with the following notes:

nturaga commented 6 years ago

Hi @MalteThodberg

Please fix the indentation in your code. An easy fix is to use formatR.

formatR::tidy_dir(path = "CAGEfightR/R")

As a reviewer it's very hard for me to review your code as it renders like this,

screen shot 2018-04-05 at 9 42 27 am

This is a Bioconductor recommended format, http://bioconductor.org/developers/how-to/coding-style/.

bioc-issue-bot commented 6 years ago

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

adcf3c4 Reformated everything with formatR d4a38a3 Merge branch 'master' of github.com:MalteThodberg/...

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

MalteThodberg commented 6 years ago

I've reformatted the entire package with formatR as you specified.

nturaga commented 6 years ago

CAGEfightR review

R CMD BUILD

ok

R CMD INSTALL

ok

DESCRIPTION

NAMESPACE

Vignettes

R

R/assign.R
184:    rm(antisense)
278:    rm(TSSs, extendedGeneModels)

R/cluster.R
54: rm(coverage_stranded)

R/combine.R
67: rm(os)
74: rm(shared_assays)
81: rm(shared_cols)
94: rm(new_meta)

R/enhancers.R
25: rm(pooled)
33: rm(P)
41: rm(covByStrand, M)
137:    rm(cw)
171:    rm(A, B)
183:    rm(scores, peaks, balance, viewsA, viewsB)
194:    rm(o_ids)
278:    rm(mat_plus, mat_minus)

R/quantify.R
173:    rm(plus_strand, minus_strand)

R/shape.R
66: rm(covByStrand)
72: # rm(coverage_plus, coverage_minus)
88: rm(TCsByStrand)
94: rm(stat_plus, stat_minus)

R/track.R
58: rm(by_strand)
153:    rm(object)

R/trimTo.R
101:    rm(coverage_stranded)
108:    rm(tcs_stranded)
126:    rm(views_plus, views_minus)
138:    rm(left_plus, right_plus, left_minus, right_minus)
144:    rm(coverage_plus, coverage_minus, irl_plus, irl_minus)
231:    rm(coverage_stranded, peaks)
238:    rm(tcs_stranded)
244:    rm(coverage_plus, coverage_minus, irl_plus, irl_minus)

R/tune.R
59:     rm(steps_series)
72: rm(by_strand)
MalteThodberg commented 6 years ago

Thank you for the comments!

The use of rm is very deliberate - some of the intermediate objects are very big, so removing them as soon as they are not used anymore greatly reduces the memory usage.

I will push a new more detailed description shortly.

bioc-issue-bot commented 6 years ago

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

1074bd1 More detailed description

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

nturaga commented 6 years ago

Let me know when you want your next review, and you are done pushing changes.

MalteThodberg commented 6 years ago

I have updated the Description, so it's ready for next review.

As explained above, the use of rm() is intentional in the few functions that do the heavy lifting. It greatly improves memory usage by removing very large RleLists, IRanges and GRanges when they are no longer needed.

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 posed to this issue in the next several days.

Thank you for contributing to Bioconductor!

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/MalteThodberg.keys is not empty), then no further steps are required. Otherwise, do the following:

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

https://bioconductor.org/packages/CAGEfightR

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.