RuoxinLi commented 5 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/quon-titative-biology/scBFA 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 5 years ago

Hi @RuoxinLi

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: scBFA
Version: 0.99.91
Date: 2019-03-09
Title: A dimensionality reduction tool using gene detection pattern to mitigate noisy expression profile of scRNA-seq
Description: This package is designed to model gene detection pattern of scRNA-seq through a binary factor analysis model. This model allows user to pass into a cell level covariate matrix X and gene level covariate matrix Q to account for nuisance variance(e.g batch effect), and it will output a low dimensional embedding matrix for downstream analysis.
Authors@R:
   c(person(given = "Ruoxin",
  family = "Li",
  role = c("aut", "cre"),
  email = "uskli@ucdavis.edu"),
     person(given = "Gerald",
  family = "Quon",
  role = c("aut"),
  email = "gquon@ucdavis.edu"))
URL: https://github.com/ucdavis/quon-titative-biology/BFA
BugReports: https://github.com/ucdavis/quon-titative-biology/BFA/issues
biocViews: SingleCell, Transcriptomics, DimensionReduction,GeneExpression, ATACSeq, BatchEffect, KEGG, QualityControl
Depends: R (>= 3.5)
Imports: SingleCellExperiment,
        SummarizedExperiment,
        Seurat,
        MASS,
        zinbwave,
        stats,
Suggests:
    knitr,
    rmarkdown,
    testthat,
    Rtsne,
    ggplot2
VignetteBuilder: knitr
SystemRequirements: R(>=3.5)
RoxygenNote: 6.1.1
License: GPL-3
LazyData: true
Encoding: UTF-8

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

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

e5bea0e remove tests/testthat/.DS_Storethat shouldn't be g... dc0225d adding gitignore file 5a18dfb making dependency on R 3.6 8efcdda change import zinbwave to only import orthogonaliz...

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

bioc-issue-bot commented 5 years ago

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

34d50e8 import copula 10060bf import functions built in copula before running ex... e002de5 make example dataset smaller insimulation to speed... 1f36bda remove zinb object b39fc21 document example dataset exprdata.rda ab01500 speed up vignette a14f973 add small example scRNA-seq data 1b20df9 update package version and man pages

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

bioc-issue-bot commented 5 years ago

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

b9949e4 ignore R history files 0a67ae8 shrink example datasize to speed up 5710688 speed up example 2637cc4 adding toy dataset for simulation 022616d document toy example dataset 57b17ba version 0.99.912 b21b03d updating man pages

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

bioc-issue-bot commented 5 years ago

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

dbd9062 further shrink the size of example data used for v... d85cecb change the man page 40fe554 speed up vignette 6db9e2c change document for exprdata.rda 11873ca speed up unit test 032914b version 0.99.912

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

bioc-issue-bot commented 5 years ago

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

1574337 correct version number 0.99.914

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

bioc-issue-bot commented 5 years ago

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

882f441 revert to b21b03d83cf19c9a851cc20df8348013cab0a522 dd1b991 current version 0.99.915

bioc-issue-bot commented 5 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.

RuoxinLi commented 5 years ago

Hi, It seems my package already passed the windows and linux system check, but for OS system, it outputs the error: dependency copula is not available for package scBFA. Can you help me install it on it? Thanks!

bioc-issue-bot commented 5 years ago

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

61bbfbe remove dependency as copula 986ccbe ignore history file and Rproj file b2232ed update vignette 11e8217 version 0.99.916

bioc-issue-bot commented 5 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.

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi When checking the package, I get an error:

> scData = CreateSeuratObject(raw.data = GeneExpr,project = "sc",min.cells = 0)
Error in CreateSeuratObject(raw.data = GeneExpr, project = "sc", min.cells = 0) : 
  unused argument (raw.data = GeneExpr)
Execution halted

Please fix this issue, it seems like the argument should be counts.

Best, Marcel

RuoxinLi commented 5 years ago

Hi Marcel,

Thank you for pointing out, I think this is due to seurat's recent update. I will adjust my code accordingly ASAP.

Best Rita

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi Any updates on the status? Thanks, Marcel

RuoxinLi commented 5 years ago

Hi Marcel,

Sorry was focusing on the reviews of BFA main paper recently.

Will address it this weekend

Thanks for the reminder

Best

Rita

On Apr 26, 2019, at 2:19 PM, Marcel Ramos notifications@github.com wrote:

Hi Rita, @RuoxinLi https://github.com/RuoxinLi Any updates on the status? Thanks, Marcel

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/1054#issuecomment-487204843, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZUMCZK6OI2RV2AXKXXY4DPSNWXFANCNFSM4HALKYUA.

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi Any updates on the status of the package? Thank you. -Marcel

RuoxinLi commented 5 years ago

Hi Marcel, I am very sorry. We are busy on addressing the reviewers comments right now, but I will make updates within this week!

Sorry for the delay.

On May 7, 2019, at 12:58 PM, Marcel Ramos notifications@github.com wrote:

Hi Rita, @RuoxinLi https://github.com/RuoxinLi Any updates on the status of the package? Thank you. -Marcel

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/1054#issuecomment-490233082, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZUMC7ESC2G56UZ45IXDG3PUHNPBANCNFSM4HALKYUA.

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi It has been quite a while. Please provide any updates that you may have. Thank you, Marcel

bioc-issue-bot commented 5 years ago

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

4812af3 update diagnostic plot 75f1cc8 update README

bioc-issue-bot commented 5 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.

RuoxinLi commented 5 years ago

Hi Marcel, @LiNk-NY sorry for the late update, can we start again on the review process, I updated the new version I had so far.

Thank you.

Rita

RuoxinLi commented 5 years ago

Hi Marcel, @LiNk-NY, again I apologize for the delay of updating and we were too busy to address the reviewer's comments within given deadlines, so I missed your last comments. Can we start the review process again?

Thank you.

Rita

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi Did you push a version bump of the package? Thanks, Marcel

RuoxinLi commented 5 years ago

Hi Marcel, @LiNk-NY The version I pushed is 0.99.914

Thanks

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi

Thank you for your submission. Please see the review below. If you have any questions, post them here in the thread.

Best, Marcel

scBFA #1054

DESCRIPTION

You may want to depend on either SummarizedExperiment or SingleCellExperiment for the access functions like mcols, colData, etc.

NAMESPACE

Looks neat

vignettes

You don't need to use an envir argument for loading the data. This might confuse users. Keep it in the .GlobalEnv.

Minor:

Use the standard R code chunk for installation instructions and use eval=FALSE

R

Avoid using semicolons (;)
You can avoid repetitive code by using the [[<- instead of $ to replace values programatically (see R/BFA.R)
Avoid using print and use message instead
Document all methods available for input to the scbfa function as the method argument and diagnose function (disperType), you may also want to create a vector of methods available and use match.arg in the body of the function to match the first one by default
Use is or is.* derivatives to check class membership instead of class(x) == "xx". You can also create a helper function to extract the data matrix from these classes (and a check and error message for unsupported classes)
Data documentation looks good (R/data.R)
Why not return a standard Bioconductor object from scNoiseSim?

Minor:

No need to evaluate TRUE twice: if (modelEnv$updateCellcoef == TRUE). Simply use if (modelEnv$updateCellcoef)
Consistently use <- as assignment operator (optional)
Use identical for scalar matching of strings, e.g., identical(diagnose_feature, "dispersion")

RuoxinLi commented 5 years ago

Thanks Marcel!

On Jul 15, 2019, at 11:05 AM, Marcel Ramos notifications@github.com wrote:

Hi Rita, @RuoxinLi https://github.com/RuoxinLi Thank you for your submission. Please see the review below. If you have any questions, post them here in the thread.

Best, Marcel

scBFA #1054 https://github.com/Bioconductor/Contributions/issues/1054 DESCRIPTION

You may want to depend on either SummarizedExperiment or SingleCellExperiment for the access functions like mcols, colData, etc. NAMESPACE

Looks neat vignettes

You don't need to use an envir argument for loading the data. This might confuse users. Keep it in the .GlobalEnv. Minor:

Use the standard R code chunk for installation instructions and use eval=FALSE R

Avoid using semicolons (;) You can avoid repetitive code by using the [[<- instead of $ to replace values programatically (see R/BFA.R) Avoid using print and use message instead Document all methods available for input to the scbfa function as the method argument and diagnose function (disperType), you may also want to create a vector of methods available and use match.arg in the body of the function to match the first one by default Use is or is.* derivatives to check class membership instead of class(x) == "xx". You can also create a helper function to extract the data matrix from these classes (and a check and error message for unsupported classes) Data documentation looks good (R/data.R) Why not return a standard Bioconductor object from scNoiseSim? Minor:

No need to evaluate TRUE twice: if (modelEnv$updateCellcoef == TRUE). Simply use if (modelEnv$updateCellcoef) Consistently use <- as assignment operator (optional) Use identical for scalar matching of strings, e.g., identical(diagnose_feature, "dispersion") — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/1054?email_source=notifications&email_token=ACZUMC4KM7NMZFPKH7IME4LP7S343A5CNFSM4HALKYUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ6QACQ#issuecomment-511508490, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZUMC32BV46NSV6IS6JO6LP7S343ANCNFSM4HALKYUA.

bioc-issue-bot commented 5 years ago

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

a2c588c version 0.99.915 446dc3e return bioconductor object from scNoiseSim 064d73f remove usage of envir argument for loading the dat... 3d19169 address reviewers' comments 2d91b27 return a standard Bioconductor object from scNois... 5a89a0d updating man pages

bioc-issue-bot commented 5 years ago

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

de765ed correct version number 0.99.917 5e0e4aa loading scData using getGeneExpr function

RuoxinLi commented 5 years ago

Hi Marcel, @LiNk-NY, I corrected the scBFA R package based on your comments, however there are two points:

You can avoid repetitive code by using the [[<- instead of $ to replace values programatically (see R/BFA.R) The modification for this comment I made is replace: modelEnv$AA <- param$AA modelEnv$ZZ <- param$ZZ modelEnv$beta <- param$beta modelEnv$gamma <- param$gamma modelEnv$UU <- matrix(param$UU,ncol = 1) modelEnv$VV = matrix(param$VV,ncol = 1) to: for(vNames in c("AA","ZZ","beta","gamma")){ modelEnv[[vNames]] = param[[vNames]] } for(Intercept in c("UU","VV")){ modelEnv[[Intercept]] = matrix(param[[Intercept]],ncol = 1) } But there are other places, like in the initialization, the evaluation for every variable is different, so I keep it unchanged, is it ok?
Document all methods available for input to the scbfa function as the method argument and diagnose function (disperType), you may also want to create a vector of methods available and use match.arg in the body of the function to match the first one by default

Do you mean document the internal function like neg_loglikelihood() , gradient()? Or you mean document the parameter of scbfa like scData, numFactors, etc? Can you give me a simple example?

Thank you!

Best!

bioc-issue-bot commented 5 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 5 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.

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi

You can avoid repetitive code by using the [[<- instead of $ to replace values programatically (see R/BFA.R) The modification for this comment I made is replace: modelEnv$AA <- param$AA modelEnv$ZZ <- param$ZZ modelEnv$beta <- param$beta modelEnv$gamma <- param$gamma modelEnv$UU <- matrix(param$UU,ncol = 1) modelEnv$VV = matrix(param$VV,ncol = 1) to: for(vNames in c("AA","ZZ","beta","gamma")){ modelEnv[[vNames]] = param[[vNames]] } for(Intercept in c("UU","VV")){ modelEnv[[Intercept]] = matrix(param[[Intercept]],ncol = 1) } But there are other places, like in the initialization, the evaluation for every variable is different, so I keep it unchanged, is it ok?

That's okay, thanks for making the change.

Document all methods available for input to the scbfa function as the method argument and diagnose function (disperType), you may also want to create a vector of methods available and use match.arg in the body of the function to match the first one by default

Do you mean document the internal function like neg_loglikelihood() , gradient()? Or you mean document the parameter of scbfa like scData, numFactors, etc? Can you give me a simple example?

This means that the diagnose function should look like:

diagnose <- function(scData,
    sampleInfo = NULL, disperType = c("Fitted", "Map", "GeneEst"),
    diagnose_feature="dispersion")
{
match.arg(disperType, c("Fitted", "Map", "GeneEst"))
...
}

So that your users can see all the possible inputs. I would also add an error message to disperType not supported. The same would apply to the scbfa function. Consider also matching case between the main function and the package name, i.e., scBFA::scBFA.

Best, Marcel

bioc-issue-bot commented 5 years ago

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

effad22 version 0.99.918 7ed992c check input strings using match.arg() a360ef1 check input strings using match.arg() and rename s... 49b2bfa update

bioc-issue-bot commented 5 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.

RuoxinLi commented 5 years ago

Hi Marcel, @LiNk-NY, I finished the changes based on your previous comments.

Thanks

Best

Rita

LiNk-NY commented 5 years ago

Hi Rita, @RuoxinLi Thank you for your contribution. Your package has been accepted.

As you continue to maintain your package please include the minor detail below:

Add the list of possible inputs in the function arguments as provided in the example above. The argument should look like this: disperType = c("Fitted", "Map", "GeneEst").

Best regards, Marcel

bioc-issue-bot commented 5 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 5 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/RuoxinLi.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 BiocManager::install("scBFA"). The package 'landing page' will be created at

https://bioconductor.org/packages/scBFA

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

submitting scBFA package #1054

scBFA #1054

DESCRIPTION

NAMESPACE

vignettes

R