proDA - Githubissues

const-ae 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/const-ae/proDA

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 @const-ae

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: proDA
Type: Package
Title: Differential Abundance Analysis of Label-Free Mass Spectrometry Data
Version: 0.99.0
Authors@R: c(person("Constantin", "Ahlmann-Eltze", email = "artjom31415@googlemail.com", role = c("aut", "cre")),
   person("Simon", "Anders", email="s.anders@zmbh.uni-heidelberg.de", role="ths"))
Description: Account for missing values in label-free mass spectrometry data 
    without imputation. The package implements a probabilistic dropout model that
    ensures that the information from observed and missing values are properly 
    combined. It adds empirical Bayesian priors to increase power to detect
    differentially abundant proteins.
License: GPL-3
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1
Suggests: 
    testthat (>= 2.1.0),
    MSnbase,
    dplyr,
    stringr,
    readr,
    tidyr,
    tibble,
    limma,
    DEP,
    numDeriv,
    pheatmap,
    knitr,
    rmarkdown
Imports: 
    stats,
    utils,
    methods,
    BiocGenerics,
    SummarizedExperiment,
    S4Vectors,
    extraDistr
URL: https://github.com/const-ae/proDA
BugReports: https://github.com/const-ae/proDA/issues
biocViews: Proteomics, MassSpectrometry, DifferentialExpression,
    Bayesian, Regression, Software, Normalization, QualityControl
VignetteBuilder: knitr

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:

18fed67 Fix reference to old package name 4c5e437 Bump version 9711fb3 Update pkgdown website

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: "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:

551bcee Remove proDA.Rproj from git repo 0165fc1 Minor fixes in documentation ea18c54 Bump version

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

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

fc3cc95 Lot's of small improvements to documentation a7e2d91 Explicitly call %||% from rlang package in build_s... a9b9042 Fix examples for generate_synthetic_data() and mpl... cab3a5d Improve text in vignettes ec4835e Bump version fec3bba Update pkgdown website

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

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

bb15b47 Fix typos with spell_check() 1bdd6dc Bump version

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

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

36bb023 Add link to preprint to README and Introduction vi... 6503c3f Bump version bc9d6ef Update pkgdown website

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: "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.

const-ae commented 5 years ago

Ahem... I have to admit I am a little confused about this error, because I actually only edited the text in the README and didn't change any code.

The error message is:

syn_data <- generate_synthetic_data(n_proteins = 10) fit <- proDA(syn_data$Y, design = syn_data$groups) Error in get(name, envir = asNamespace(pkg), inherits = FALSE) : object 'normalize_names_replacement_value' not found

From the error message, I presume that it is related to the recent change in S4Vectors package by @hpages.

I am not quite sure, what the correct way forward is now. Can I somehow change my code, so that no more error is thrown or should I just wait and bump the version at some point to trigger a rebuild?

I would appreciate any input :)

bioc-issue-bot commented 5 years ago

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

1a2379f Bump version to trigger 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.

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

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

3f4eadf Bump version and update pkgdown website

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.

Kayla-Morrell commented 5 years ago

Hello @const-ae,

Thank you for submitting to Bioconductor, I'm sorry for the delay. Please see the initial review of the package below. Comment back here with updates that have been made and when the package is ready for a re-review.

General package development

[ ] REQUIRED: Please take a look at unnecessary files in the repo, particularly the doc/ directory, _pkgdown.yml, and the inst/site directory. These files may be present in your local directory but should not be commited to git. If they serve a purpose in the repo than please justify.

DESCRIPTION

[ ] REQUIRED: 'LazyData:' should be set to false. If this field being true can slow down the loading of the package.

NAMESPACE

[ ] SUGGESTION: Generally 'importFrom()' is encouraged over importing an entire package. If there are many functions being used from the package than 'import()' is okay. Just check to make sure its necessary to import methods, stats and utils.

Data

[ ] REQUIRED: There needs to be documentation for 'proteinGroups.txt' in the inst/extdata directory. The documentation can be put in an inst/script/ directory.

Vignette

Introduction

[ ] REQUIRED: The installation section should show users how to install the package from Bioconductor using the following code:

if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("proDA")

[ ] REQUIRED: Line 67, this code needs to be recognized as R code so be sure to add '{ }' around the r. Also include 'eval = FALSE' so this section of code isn't actually run.
[ ] SUGGESTION: Line 88 and 89, you advise new users to skip right to section two for the complete walkthrough. Technically section 2 is your Session Info section so you may want to advise them to skip to section 1.3 where the walkthrough begins.
[ ] REQUIRED: Line 333, this image should be put in the vignette directory since there shouldn't be images in the man directory.

Man pages

[ ] REQUIRED: The folder 'figures' should not be in the man directory.

inst/script/

[ ] REQUIRED: The scripts in this directory can vary. Most importantly if data was included in the inst/extdata/, a related script must be present in this directory documenting very clearly how the data was generated. It should include source urls and any important information regarding filtering or processing. It can be executible code, sudo code, or a text description. A user should be able to download and be able to roughly reproduce the file or object that is present as data. It seems like you have the start of this file in the 'data-import' vignette.

R code

[ ] REQUIRED: Avoid sapply(); use vapply() instead. Found in files:
- util.R, line 66
[ ] SUGGESTION: For formatting reasons, consider shorter lines. There are 269 lines that are > 80 charactes long.
[ ] SUGGESTION: For formatting reasons, consider multiples of 4 spaces for line indents. There are 1333 lines that are not.

Best, Kayla

bioc-issue-bot commented 5 years ago

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

7835033 Change LazyData to false in DESRIPTION 44b4b5d Handle spike-ins in median_normalization Add a ne... a58eee2 Don't import all utils functions, but only the nec... ba742b3 Add ORCIDs to DESCRIPTION b88caa9 Bump version

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

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

4ebb40a Update installation instructions 7a6e5c9 Move figures folder from man/ to vignettes/ and up... 88767e6 Fix typo in data-import.Rmd 14f14ee Add file in inst/script that describes where the p... e30b5f0 Bump version

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.

const-ae commented 5 years ago

Hi @Kayla-Morrell,

thank you very much for your thorough review and helpful comments. I am sorry that it now took me so long to address your comments and fix the issues. Below I will go through each of your points and explain how I addressed them or add further explanation why did something the way I did.

REQUIRED: 'LazyData:' should be set to false. If this field being true can slow down the loading of the package.

Fixed.

REQUIRED: The installation section should show users how to install the package from Bioconductor using the following code [...] REQUIRED: Line 67, this code needs to be recognized as R code so be sure to add '{ }' around the r. Also include 'eval = FALSE' so this section of code isn't actually run.

Fixed.

SUGGESTION: Line 88 and 89, you advise new users to skip right to section two for the complete walkthrough. Technically section 2 is your Session Info section so you may want to advise them to skip to section 1.3 where the walkthrough begins.

Fixed.

REQUIRED: Line 333, this image should be put in the vignette directory since there shouldn't be images in the man directory. REQUIRED: The folder 'figures' should not be in the man directory.

Fixed.

REQUIRED: Please take a look at unnecessary files in the repo, particularly the doc/ directory, _pkgdown.yml, and the inst/site directory. These files may be present in your local directory but should not be commited to git. If they serve a purpose in the repo than please justify.

Those files are for the pkgdown website at https://const-ae.github.io/proDA/. Pkgdown automatically turns the vignettes and the man pages into html so that I can link to specific functions and people can avoid downloading the vignette. The html pages are hosted by github, so it is necessary for me to commit them to the repository. Thus, if at all possible, I would like to keep them.

SUGGESTION: Generally 'importFrom()' is encouraged over importing an entire package. If there are many functions being used from the package than 'import()' is okay. Just check to make sure its necessary to import methods, stats and utils.

I removed the utils package, because I didn’t actually use many functions from it. In contrast, functions from the stats package are used through out the code.

REQUIRED: There needs to be documentation for 'proteinGroups.txt' in the inst/extdata directory. The documentation can be put in an inst/script/ directory. REQUIRED: The scripts in this directory can vary. Most importantly if data was included in the inst/extdata/, a related script must be present in this directory documenting very clearly how the data was generated. It should include source urls and any important information regarding filtering or processing. It can be executible code, sudo code, or a text description. A user should be able to download and be able to roughly reproduce the file or object that is present as data. It seems like you have the start of this file in the 'data-import' vignette.

I added a proteinGroups_information.txt file in the inst/scripts folder. The dataset that I use in the R package for demonstration purposes is not yet publicly available, so I cannot actually provide instruction how download it, but I describe how it was generated, the subsetting step and general overview of the experiment.

REQUIRED: Avoid sapply(); use vapply() instead. Found in files: util.R, line 66

I use sapply() in the msply_dbl() function that explicitly validates the output and makes sure that always a numeric matrix is returned. Using vapply() would be difficult in this context, because I don’t know how many elements the function FUN returns. Is it under those circumstances possible that I keep using sapply()?

SUGGESTION: For formatting reasons, consider shorter lines. There are 269 lines that are > 80 charactes long. SUGGESTION: For formatting reasons, consider multiples of 4 spaces for line indents. There are 1333 lines that are not.

If necessary I can go through the whole repo and change the spacing and line breaks, but if possible I would prefer to keep it this way.

I hope that I didn't forget any point. I am looking forward to your feedback. Best Regards, Constantin

bioc-issue-bot commented 5 years ago

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

9307bb7 Bump version

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.

Kayla-Morrell commented 4 years ago

@const-ae - Thank you for making the necessary changes and providing further explanation for some points above. Everything looks good now and I'm happy to accept the package!

Best, Kayla

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!

const-ae commented 4 years ago

@Kayla-Morrell, thanks that is great 🎉 Thank you again for your feedback and helpful comments 😃

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/const-ae.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("proDA"). The package 'landing page' will be created at

https://bioconductor.org/packages/proDA

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

proDA #1137

General package development

DESCRIPTION

NAMESPACE

Data

Vignette

Man pages

inst/script/

R code