Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

scPipe: a package for single cell RNA-seq data processing #431

Closed LuyiTian closed 7 years ago

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

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: scPipe
Title: pipeline for single cell RNA-seq data analysis
Version: 0.99.0
Type: Package
Maintainer: Luyi Tian <tian.l@wehi.edu.au>
Author: Luyi Tian
Authors@R: c(person("Luyi", "Tian", role=c("aut", "cre"),
        email="tian.l@wehi.edu.au"), person("Shian", "Su",
        role=c("aut"), email="su.s@wehi.edu.au"), person("Matt",
        "Ritchie", role=c("ctb"),
        email="mritchie@wehi.edu.au"),  person("Shalin",
        "Naik", role=c("ctb"),
        email="naik.s@wehi.edu.au"))
biocViews: Software, Sequencing, RNASeq, GeneExpression, SingleCell, Visualization,
        SequenceMatching, Preprocessing, QualityControl, GenomeAnnotation
Description: to process single cell RNA-seq data from fastq to gene counting matrix. it can process data generated by CEL-seq, MARS-seq, Drop-seq and SMART-seq.
Depends: R (>= 3.4), Biobase, ggplot2, methods
LinkingTo: Rcpp, Rhtslib, zlibbioc
Imports: biomaRt, GGally, MASS, mclust, Rcpp (>= 0.11.3), reshape, robustbase, Rsubread, scales, utils, Rtsne, destiny, stats 
License: GPL (>= 2)
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.0.1
NeedsCompilation: yes
URL: https://github.com/LuyiTian/scPipe
BugReports: https://github.com/LuyiTian/scPipe
Suggests: knitr,
    rmarkdown
VignetteBuilder: knitr
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/scPipe_buildreport_20170728053007.html

LuyiTian commented 7 years ago

I have updated the package to fix errors: https://github.com/LuyiTian/scPipe/commit/cc3260078c854c94f28cef1fda4ee955e43db6ed

also added the Webhook to my repo, but the first delivery is failed with error: "Only push, issue, and issue comment event hooks supported". I have selected "Just the push event."

bioc-issue-bot commented 7 years ago

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

bcbf9ed new version number for bioc auto 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/scPipe_buildreport_20170801043051.html

bioc-issue-bot commented 7 years ago

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

b36c6a6 try

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

bioc-issue-bot commented 7 years ago

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

08d96b1 m

lawremi commented 7 years ago

There seem to be a lot of partially overlapping packages for single cell RNA-seq. Rather than package up an entire workflow, why not release each of these steps as a separate package? Users could then mix and match algorithms to construct pipelines. It's fine to publish a workflow package, but it should probably just combine other, independent components. With the convergence towards common data structures like SingleCellExperiment, it seems like there is now a critical mass for these packages to start working towards modular interoperability.

Btw, this should probably generate a SingleCellExperiment, or at least a SummarizedExperiment.

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, 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/scPipe_buildreport_20170801085824.html

bioc-issue-bot commented 7 years ago

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

70ab31c update

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

LuyiTian commented 7 years ago

@lawremi Thanks for your advice.

Yes I agree there are a huge amount of scRNA-seq packages now but instead of the whole workflow, scPipe only tries to fill the gap between fastq and gene counting matrix and QC information, generating the consistent result for different protocols. It does the similar things as Cellranger does for 10X, but works for all the scRNA-seq protocols like CEL-seq and Drop-seq.

I have an SCData object, designed in a similar way as SCE in scater and the workflow does generate an SCData object in the end. But now I am working on transforming all the SCData related functions to SingleCellExperiment since more packages are gonna use it in the near future.

LuyiTian commented 7 years ago

@vobencha @lawremi My package does not work on windows because of the C++ code. Could you tell me how do I suppress the build on windows platform? Currently, the only error comes from the windows. Thank you very much.

vobencha commented 7 years ago

Suppressing the build on windows is not an option. The package must build on all 3 platforms - this is part of the new package requirements. Valerie

bioc-issue-bot commented 7 years ago

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

919e906 Try to fix the errors on windows

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, 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/scPipe_buildreport_20170803011318.html

bioc-issue-bot commented 7 years ago

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

7b6d049 update

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

bioc-issue-bot commented 7 years ago

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

c0cf65b try to fix the win build 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: "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/scPipe_buildreport_20170804040257.html

LuyiTian commented 7 years ago

@vobencha We are working on the code so it could be built on win. But is it possible to limit my package to only linux the Mac, because my package contains a lots of C++ code which compiles at unix but failed in win platform by mingw_64.

vobencha commented 7 years ago

I won't be doing a full review until the package passes build and check on all 3 platforms. As I mentioned before, the requirements are that that package build on all 3 platforms or it won't be accepted into the BioC repository. You are welcome to submit to CRAN if you can't make it build on windows. Valerie

bioc-issue-bot commented 7 years ago

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

50160a0 new version number fix the win build 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/scPipe_buildreport_20170811054543.html

bioc-issue-bot commented 7 years ago

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

4994065 version number

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

bioc-issue-bot commented 7 years ago

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

f79d6e6 Update readme. Put run_scpipe back

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

bioc-issue-bot commented 7 years ago

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

816607a Fix the Rsubread issue

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

bioc-issue-bot commented 7 years ago

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

972bbd6 have to put organism and count back because of war...

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

bioc-issue-bot commented 7 years ago

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

db43fab add try catch in biomart database connection some...

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

vobencha commented 7 years ago

Hi,

I've taken a look at the package. I think we should postpone review until you've switched the code over to use SingleCellExperiment or scatter::SCESet as appropriate and removed the SCData structure.

I also think the package should be renamed to something more descriptive. scPipe doesn't say much about what the package does. You said you're focusing on "the gap between fastq and gene counting matrix and QC information, generating the consistent result for different protocols.". Maybe something along the lines of SingleCellQC?

Some general comments on package development:

1) Format the code to be <= 80 characters wide.

2) man page examples and /dontrun In general, all man pages need examples and should not have /dontrun statements. Even if this is pipeline small examples of relavent functions should be shown. If you feel there are special cases these will need to be explained.

3) Inconsistent naming of package functions and classes

Currently there is a mixture of camelCase and underscores. Please choose one consistent name scheme:
~//scPipe/man >ls
calQCMetrics.Rd    create_report.Rd      FACSData.Rd         merge_SCData.Rd  plotQC_pair.Rd      sc_celseq2_simulator.Rd  sc_detect_bc.Rd      scPipe.Rd           tpm.Rd
convert_geneid.Rd  create_scd_by_dir.Rd  fpkm.Rd             newSCData.Rd     QCMetrics.Rd        SCData.Rd                sc_dim_reduction.Rd  sc_sample_data.Rd
counts.Rd          detect_outlier.Rd     gene_id_type.Rd     organism.Rd      remove_outliers.Rd  SCData-subset.Rd         sc_exon_mapping.Rd   sc_sample_qc.Rd
cpm.Rd             DimReducedExpr.Rd     get_genes_by_GO.Rd  plot_mapping.Rd  run_scPipe.Rd       sc_demultiplex.Rd        sc_gene_counting.Rd  sc_trim_barcode.Rd

4) S4 methods S4 methods in R do not use the dot '.' for dispatch. It looks like you've created helpers (named with dot) for your accessors and then exported them. Usually helpers are created when code is reused by several functions or something rather complicated is going on. I see neither case here.

For example, I don't see the point of creating counts.SCData(); instead the one-liner could just be part of counts(). If you feel you need the helper then it should be internal only and not exported.
counts.SCData <- function(object) {
  object@assayData$counts
}
setMethod("counts", signature(object = "SCData"), counts.SCData)

These should not be exported:

~/repos/git/software/scPipe >grep '\.SCData' NAMESPACE
export(FACSData.SCData)
export(QCMetrics.SCData)
export(counts.SCData)
export(fpkm.SCData)
export(gene_id_type.SCData)
export(organism.SCData)
export(tpm.SCData)

5) Instead of dir.create() in the vignette user tempdir().

6) general tidy The package has many typos and does not use capitalization for complete sentences. For example, here is lack of capitalization on the convert_geneid.Rd man page:

\item{returns}{the gene id which is set as return. default to be `external_gene_name`
A possible list of attributes can be retrieved using the
function \code{listAttributes} from \code{biomaRt} package. the commonly used
id types are `external_gene_name`, `ensembl_gene_id` or `entrezgene`.}

\item{all}{logic. for genes that cannot covert to new gene id, keep them with the old
id or delete them. the default is keep them.}

Typos on convert_geneid.Rd:
The argument name is 'returns' but the example uses 'return':

scd = convert_geneid(scd, return="entrezgene")

Please review the man pages and vignette for typos, capitalization and code format (max 80 characters wide).

I would like to close this issue and have you open a new issue once SingleCellExperiment has been incorporated and the package has been renamed.

Valerie

bioc-issue-bot commented 7 years ago

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

77eb00e remove Rsubread in suggests to fix the 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/scPipe_buildreport_20170815214908.html

LuyiTian commented 7 years ago

@vobencha Thank you for your comment. Your suggestions are very helpful, though I would be conservative about the package naming.

I will close the issue and resubmit it soon.

bioc-issue-bot commented 7 years ago

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

ad3abb4 New version