search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

gscreend package #1228

Closed imkeller closed 4 years ago

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

Hi @imkeller

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: gscreend
Type: Package
Title: Analysis of pooled genetic screens
Version: 0.1.1
Author: Katharina Imkeller
Maintainer: Katharina Imkeller <k.imkeller@dkfz.de>
Description: Package for the analysis of pooled genetic screens (e.g. CRISPR-KO). The analysis of such screens is based on the comparison of gRNA abundances before and after a cell proliferation phase. The gscreend packages takes gRNA counts as input and allows detection of genes whose knockout decreases or increases cell proliferation.
License: GPL-3
Encoding: UTF-8
LazyData: true
Depends: R (>= 3.6)
Imports: SummarizedExperiment, nloptr, fGarch, methods
Suggests: knitr
VignetteBuilder: knitr
RoxygenNote: 6.1.1.9000
biocViews: Software, StatisticalMethod, PooledScreens, CRISPR

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

0db8bcc Version 0.99.1; removed unwanted files from git tr...

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

444f5cf version 0.99.2

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:

dae1f0d Do not export preprocessing functions. Added resul... f270162 Do not export preprocessing functions. Added resul...

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.

nturaga commented 4 years ago

Initial review

Please correct the NOTES in your build report,

* Checking coding practice...
    * NOTE: Avoid sapply(); use vapply()
      Found in files:
        GeneRanking.R (line 33, column 20)
        GeneRanking.R (line 61, column 5)
    * NOTE: Avoid 1:...; use seq_len() or seq_along()
      Found in files:
        GeneRanking.R (line 12, column 37)
        GeneRanking.R (line 12, column 47)
        GeneRanking.R (line 17, column 24)
        QualityControl.R (line 40, column 22)
        sgrnaRanking.R (line 78, column 15)
        sgrnaRanking.R (line 105, column 15)
* Checking package NEWS...
    * NOTE: Consider adding a NEWS file, so your package news will be
      included in Bioconductor release announcements.
* Checking unit tests...
    * NOTE: Consider adding unit tests. We strongly encourage them. See
      http://bioconductor.org/developers/how-to/unitTesting-guidelines/.
    * NOTE: Consider shorter lines; 17 lines (2%) are > 80 characters
      long.
    First 6 lines:
      R/DataInput.R:31         message("Creating PoolScreenExp object from a ...
      R/DataInput.R:34         message("Error: input data is not of the supor...
      man/PoolScreenExp-class.Rd:15 \item{\code{sgRNAData}}{A SummarizedExper...
      man/PoolScreenExp-class.Rd:17 \item{\code{FittingIntervals}}{A vector d...
      man/PoolScreenExp-class.Rd:20 \item{\code{LFCModelParameters}}{A vector...
      man/PoolScreenExp-class.Rd:23 \item{\code{GeneData}}{SummarizedExperime...
    * NOTE: Consider multiples of 4 spaces for line indents, 45
      lines(4%) are not.
    First 6 lines:
      R/core.R:41                           quant1 = 0.1,
      R/core.R:42                           quant2 = 0.9,
      R/core.R:43                           alphacutoff = 0.05,
      R/core.R:44                           n_cores=1) {
      R/core.R:51                           quant1, quant2)
      R/GeneRanking.R:82                      replicate(n_repl,
    See http://bioconductor.org/developers/how-to/coding-style/
    See FormatR package:
      https://cran.r-project.org/web/packages/formatR/index.html
bioc-issue-bot commented 4 years ago

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

4ae5212 Removed unnecessary number of cores parameter. Bio... cb96eec Updated stop/message functions. Testing for correc... 01ff466 Version 0.99.4

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.

imkeller commented 4 years ago

Dear @nturaga , Thank you for the initial review. I corrected the NOTES in the build report. Based on what I saw for in the reviews of other packages I also replaced 'parallel' by 'BiocParallel' package.

Kayla-Morrell commented 4 years ago

Hello @imkeller,

Thank you for submitting to Bioconductor. Please see the review of the package below. The required changes must be made while the suggested changes do not have to be but we strongly encourage them. Comment back here with updates that have been made and when the package is ready for a re-review.

DESCRIPTION

NAMESPACE

Vignettes

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

Then I would include the R code chunk from the first vignette requirement above (lines 14-17). This will then show the user how to install and load the package using Bioconductor.

res <- ResultsTable(pse_an, direction = "negative")
head(res)

Man pages

inst/script/

Unit tests

R code

Overall your code is very well written and organized!

Best, Kayla

bioc-issue-bot commented 4 years ago

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

cc9836e Added very basic unit tests. c1e7421 Version 0.99.5

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.

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

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

e0e4d7f Version 0.99.6 Fixed issues with export of accesso...

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.

imkeller commented 4 years ago

Hi @Kayla-Morrell ,

Thank you for the review and your suggestions. I implemented all of them in the new version that I just pushed to github. Below this message I add a detailed response to your comments. The package is ready for a re-review.

Many thanks, Katharina

DESCRIPTION

REQUIRED: 'LazyData' should be set to false. If this field is true it can slow down the loading of packages. Done

SUGGESTION: It is encouraged to include the field 'BugReports:' with the relevant link to GitHub for reporting Issues. Done

NAMESPACE

REQUIRED: The PoolScreenExp class should be exported. Done

SUGGESTION: Generally 'importFrom()' is encouraged over importing an entire package, however if there are many functions from a single package, 'import()' is okay. Done

Vignettes

REQUIRED: Lines 14-17, I would move this code into the 'Installation' section. I will explain further in a later requirement point. Done

REQUIRED: Lines 34-44, in order for these lines to become an ordered list there needs to be a blank line between line 33 and line 34. Without that, the list just looks like a paragraph of text. Done

REQUIRED: The 'Installation' section should demonstrate to the user how to download the package from Bioconductor. I would include the following code with eval = FALSE, if(!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager") BiocManager::install("gscreend") Then I would include the R code chunk from the first vignette requirement above (lines 14-17). This will then show the user how to install and load the package using Bioconductor. Done

SUGGESTION: When talking about specific functions you can put around the function to indicate it as R code. So in line 121, plotModelParameters() could become plotModelParameters(). It helps to differentiate it from regular text. Done

REQUIRED: Line 138, to print out the whole table is just a waste of space in the vignette. I would save this function call to a variable and then just print the head of the variable. This will give the user an idea of what the data looks like without having to print out the whole table. Something like... res <- ResultsTable(psean, direction = "negative") head(res) Done_

REQUIRED: The last section of the vignette should be 'Session Info' and should include sessionInfo(). Done

Man pages

SUGGESTION: Only functions that are exported need to have man pages. The functions that do have man pages but aren't exported can get the tag @keywords internal added to them so that the function is removed from the documentation index. I added @keywords internal to the function description (e.g. assignGeneData()) and ran devtools::document(). I am not sure, whether the result I get is the intended one, because the corresponding entries in man/ are still there.

inst/script/

REQUIRED: Since there is data included in 'inst/extdata/' there should be a related script in this directory documenting how the data was generated. It should include source urls and any important information regarding filtering or processing. It can be executable code, sudo code, or a text description. A user should be able to download and roughly reproduce the file or object that is present as data. Done

Unit tests

SUGGESTION: Consider adding unit test. We strongly encourage them. See http://bioconductor.org/developers/how-to/unitTesting-guidelines/. I added unit tests for some of the functions, especially related to data input and quality control.

R code

Overall your code is very well written and organized! Thanks :)

REQUIRED: Avoid the use of direct slot access with @ or slot(). Accessor methods should be created and utilized instead. I removed the slot access via @ from most of the code, except the accessor methods.

Kayla-Morrell commented 4 years ago

@imkeller - Thank you for making the changes, everything looks good 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!

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/imkeller.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 BiocManager::install("gscreend"). The package 'landing page' will be created at

https://bioconductor.org/packages/gscreend

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.