FrederickHuangLin commented 4 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/FrederickHuangLin/ANCOMBC

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

Hi @FrederickHuangLin

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: ANCOMBC
Type: Package
Title: Analysis of compositions of microbiomes with bias correction
Version: 0.99.0
Description: ANCOMBC is a package for normalizing the microbial absolute 
    abundance data and identifying taxa that are differentially abundant with 
    respect to the covariate of interest.
Date: 2020-08-10
Authors@R: c(
    person(given = "Huang", 
 family = "Lin", 
 email = "HUL40@pitt.edu", 
 role = c("cre", "aut"),
 comment = c(ORCID = "https://orcid.org/0000-0002-4892-7871")),
    person(given = c("Shyamal", "Das"), 
 family = "Peddada", 
 email = "Shyamal.Peddada@nih.gov", 
 role = "aut",
 comment = c(ORCID = "https://orcid.org/0000-0002-5014-6513")))
License: Artistic-2.0
LazyData: true
Imports: stats, MASS, nloptr, Rdpack
Suggests: knitr, microbiome, tidyverse, testthat, DT, magrittr, qwraps2
biocViews: 
    DifferentialExpression, 
    Microbiome, 
    Normalization, 
    Sequencing, 
    Software
BugReports: https://github.com/FrederickHuangLin/ANCOMBC/issues
URL: https://github.com/FrederickHuangLin/ANCOMBC
VignetteBuilder: knitr
RdMacros: Rdpack
Encoding: UTF-8
RoxygenNote: 7.1.1

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 this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ANCOMBC to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 4 years ago

Hi Frederick, @FrederickHuangLin

Thank you for your submission. Please see the review below.

Best, Marcel

ANCOMBC #1586

The package does not seem to interface with any existing Bioconductor infrastructure
This is a challenge because there is less to 're-use' in Bioconductor that is related to the analyses that this package performs
On that note, why shouldn't it go to CRAN?

DESCRIPTION

Include more information in the Description: field to give users a sense of the scope of the package
Your ORCID IDs should include just the numbers at the end of the URL.
Remove the LazyData: true field, your package does not seem to include data
Perhaps Rdpack should be in Suggests:

NAMESPACE

Looks pretty clean

vignettes

(optional) You could add is a # Reference section at the bottom when using https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html
Avoid using backticks in the recode function unless the name has a special character (also in tests/testthat/*)
Use TRUE instead of T
Are users expected to write summary_template? It may be more helpful to provide this object via a helper function
Avoid using the semi-colon ; and add a new line
Use the group of assignments (lines 133 - 139) inside the ancombc function call with proper indentation and argument names for readability
You may want to add a small description of the ANCOMBC primary result informing the reader what kind of return is to be expected.
Everything else looks good.

R

Avoid using ; in package code as well
Clean up objects that are not used after creation (e.g., taxa_id, samp_id in R/ancombc.R)
General Note (No action required). It seems from the coding that it can be tough to keep track of every object that is created. It's better to create a general class that work with functions that would only extract the information that is needed.
Perhaps algorithms such as E-M are implemented elsewhere and a re-implementation may not be necessary?
Good use of vapply
Avoid, where possible, nested apply family calls (e.g., in R/get_struc_zero.R)
Minor: Avoid using c() on what should already be a vector (in R/em_iter.R; L#4-7)
Use <- instead of = for assignment (R/fit_summary.R)
You can use on.exit to reset options in para_est
Avoid constructions like ifelse(res$q_val < alpha, TRUE, FALSE) which can be res$q_val < alpha & !is.na(res$q_val) if there are NA values otherwise the first condition (res$q_val < alpha) should be enough

tests

Looks good

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: a3ab72fa15c2e226150ed0fe22cfc391b8b0791a

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ANCOMBC to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

FrederickHuangLin commented 4 years ago

Hi Marcel, @LiNk-NY

Thank you so much for your review! I really appreciate it!

I have made changes accordingly and pushed to upstream. Here are my responses:

ANCOMBC

Packages from Bioconductor are more widely used by the bioinformatics community. Since microbiome data is an emerging omics data, we believe releasing on Bioconductor would be more suitable. Also, we plan to submit another package called ANCOM to Bioconductor as well, which will be highly correlated to the analyses that ANCOMBC performs.

DESCRIPTION

Done
Done
Done
I followed the Rdpack setup from its user manual

NAMESPACE

Thank you

vignettes

(optional) Done
Done
Done
summary_template is very specific to the data set used in the vignettes, which means it is not able to generalize to other data sets. That's why we did not write it as a helper function
Done
Done
Done
Thank you

R

Done
Done
Thank you for your suggestion
The E-M algorithm implemented in ancombc requires a little "twist" since the number of distribution components is three and variances of the central distribution are considered as known. That's why we would prefer to write the E-M algorithm on our own.
Thank you
Done
Done
Could you please elaborate on why specifically I should replace = with <- for assignment in R/fit_summary.R?
Thank you for your suggestion. A new internal function get_x.R has been added
Done

tests

Thank you

Best regards,

Frederick

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ANCOMBC to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 4 years ago

Hi Frederick, @FrederickHuangLin

Packages from Bioconductor are more widely used by the bioinformatics community. Since microbiome data is an emerging omics data, we believe releasing on Bioconductor would be more suitable. Also, we plan to submit another package called ANCOM to Bioconductor as well, which will be highly correlated to the analyses that ANCOMBC performs.

We recommend that you look for integration opportunities for the current package before submitting another one. Packages such as phyloseq, microbiome may provide some direction.

We also think that you can use SummarizedExperiment as your main representation in the package. This will possibly allow packages such as phyloseq to integrate with yours.

Best, Marcel

FrederickHuangLin commented 4 years ago

Hi Marcel, @LiNk-NY,

Thanks a lot for your suggestions!

I will change the input object as phyloseq-class experiment-level object as it is used in both phyloseq and microbiome packages. I am not quite sure about SummarizedExperiment object as we do not usually have regions-of-interest data.

If there is any other integration opportunities other than the input object, please do not hesitate to let me know, I really appreciate your help!

In the meantime, since the ANCOM-BC paper has been published and people are asking for its R package, do you think we can make it available soon, and I will make the integration of phyloseq-class experiment-level object as its next update?

Thank you,

Frederick

LiNk-NY commented 4 years ago

Hi Frederick, @FrederickHuangLin

Thanks for integrating with phyloseq. We can make it available in Bioconductor devel soon after the review process. The 3.11 release deadline is past but devel will become the next release (Bioc ~ 3.12).

Best, Marcel

lshep commented 4 years ago

Please make the updates before the package is accepted.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: bc9af3c1d9a1ce37e431da3609a0285c80a5b153

FrederickHuangLin commented 4 years ago

Please make the updates before the package is accepted.

Hi,

I have submitted the updates. However, the first time I used an inappropriate version number, I received the following error message:

remote: Error: Illegal version bump from '0.99.1' to '1.0.0'.

I corrected the version number and pushed changes to the Bioconductor again, the error message becomes:

remote: fatal: this operation must be run in a work tree
remote: ERROR:root:Exception: Command '['git', 'rev-parse', '--show-toplevel']' returned non-zero exit status 128
remote: ERROR:root:local variable 'package_name' referenced before assignment
remote: Note: failed to update RSS feed;git repository updated successfully.
To git.bioconductor.org:packages/ANCOMBC.git
   a3ab72f..bc9af3c  master -> master

In the meantime, the bot showed that I have made "a valid push".

I was wondering whether my push was successful, and how can I resolve the above error messages?

Thank you,

Frederick

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ANCOMBC to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

FrederickHuangLin commented 4 years ago

Hi Marcel @LiNk-NY, and @lshep,

Sorry if this message is duplicated. I have not heard from you since my last post. I greatly appreciate it if you could inform me whether my last push is successful; and if not, how could I address it?

Thank you,

Frederick

LiNk-NY commented 4 years ago

Hi Frederick, @FrederickHuangLin Sorry for the delay. Your commits were successfully pushed from the response I see from the bot. I am taking a look at the changes.

Could you please elaborate on why specifically I should replace = with <- for assignment in R/fit_summary.R?

This is a matter of preference for clarity. Assignment in my opinion should always be done with <- so that there is no confusion between = used for arguments and = used for assignment.

Thanks. -MR

LiNk-NY commented 4 years ago

Hi Frederick, @FrederickHuangLin

Thanks for integrating phyloseq into the package. Before acceptance, consider using an example dataset (e.g., data(GlobalPatterns)) from the package and show how the phyloseq object is used in your functions. The examples are a little disconnected. Thanks!

Best, Marcel

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 8e8e5874ca88548834b76e8259ea26f6da6f129b

FrederickHuangLin commented 4 years ago

Thank you, Marcel @LiNk-NY

A new push has been made to change the example dataset to GlobalPatterns data.

Best,

Frederick

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ANCOMBC to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 4 years ago

Hi Frederick, @FrederickHuangLin

It looks great, thanks! I will accept your package.

Best regards, Marcel

bioc-issue-bot commented 4 years ago

Your package has been accepted. It will be added to the Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

mtmorgan commented 4 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/FrederickHuangLin.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("ANCOMBC"). The package 'landing page' will be created at

https://bioconductor.org/packages/ANCOMBC

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

ANCOMBC #1586

ANCOMBC #1586

DESCRIPTION

NAMESPACE

vignettes

R

tests

ANCOMBC

DESCRIPTION

NAMESPACE

vignettes

R

tests