Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

gemini #1148

Closed ssj1739 closed 5 years ago

ssj1739 commented 5 years ago

GEMINI: A variational Bayesian approach to identify genetic interactions from combinatorial CRISPR screens

This package is being submitted to Bioconductor upon conditional acceptance of a corresponding manuscript. The contents of the package are described in detail within the manuscript, as well as with the accompanying documentation.

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

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: gemini
Type: Package
Title: GEMINI: Variational inference approach to infer genetic interactions from pairwise CRISPR screens
Version: 0.3.0
Authors@R: c(
  person("Mahdi", "Zamanighomi", email = "mzamanig@broadinstitute.org", role = c('aut')),
  person("Sidharth", "Jain", email = "sidharthsjain@gmail.com", role = c('aut', 'cre'))
  )
Description: GEMINI uses log-fold changes to model sample-dependent and independent effects,
    and uses a variational Bayes approach to infer these effects. The inferred effects
    are used to score and identify genetic interactions, such as lethality and recovery.
    More details can be found in Zamanighomi et al. at doi:???.
Depends: R (>= 3.6.0)
License: BSD_3_clause + file LICENSE
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1
biocViews: Software, CRISPR, Bayesian
Imports:
  dplyr,
  grDevices,
  ggplot2,
  magrittr,
  mixtools,
  scales,
  pbmcapply,
  parallel,
  stats,
  utils
Suggests: 
    knitr,
    rmarkdown,
    testthat
VignetteBuilder: knitr

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:

bfbcbfd Update version for prerelease 2c83601 Update version for prerelease

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

ssj1739 commented 5 years ago

Hi,

I've pushed to the linked repository, but it seems that there is no build being triggered. Additionally, the warnings in the previous build appear to be from packages not being present or installed on the host machine. Is there any way for me to fix this?

Thanks, Sidharth Jain

Kayla-Morrell commented 5 years ago

Hello @ssj1739 - in order for a build to be triggered you need to bump the version in the DESCRIPTION file. Currently it is version 0.99.0 and once you bump it to 0.99.1 and push the change then a build should be triggered. This needs to be done every time you push changes. I'll look into the warnings a bit further on our end.

Best, Kayla

bioc-issue-bot commented 5 years ago

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

b15779c Updated package with DOI from Zenodo Incremented v... 1a3f6db Merge branch 'master' of github.com:sellerslab/gem...

ssj1739 commented 5 years ago

@Kayla-Morrell Thank you, I'll be sure to do that from now on.

I think the warnings may have to do with documentation - in the documentation of some functions, I'm linking to packages that aren't in the "Imports", but are instead in "Suggests", and I'm not sure if the machines that install packages are also installing all suggested packages. I can remove this documentation, but I'm wondering what Bioconductor's best practices are regarding linking in documentation.

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

7732131 Changed documentation to increase vague-ness of re... 8651bcd Increment version to 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: "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:

3dff5a9 Removing references to mclapply and magrittr as mu...

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

14f1f48 Increment to version 0.99.4

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:

c4d5149 Increment to version 0.99.5 - add badges to 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.

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

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

fc7a66a Removed segmented package from Imports. Incremente...

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

cab5547 Removed link to magrittr package in compound pipe ... 685a100 Increment version to 0.99.7

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

8310041 Fixed syntax error in compound.Rd ea76f60 Increment version to 0.99.8

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

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

DESCRIPTION

NAMESPACE

vignettes

BiocManager::install("gemini")



- [ ] SUGGESTION: We strongly encourage the use of table of contents.

- [ ] REQUIRED: Please include some text that explains where the Big Papi data 
comes from.

- [ ] SUGGESTION: For the sections using `knitr::kable()`, I would change the 
'echo = FALSE' to true so that the user can see how to achieve the nice table 
you produce. Especially when you start subsetting by A549 (line 111, 117), the 
user may not know how to do this and showing the code would make the code 
reproducable.

- [ ] REQUIRED: Line 122, since 'eval = FALSE' the plot is not produced in the 
vignette. So when you say "We can see that GEMINI..." we can't because the plot
 is not there. Please remove the 'eval = FALSE'. 

- [ ] REQUIRED: Line 137, the same goes for this chunk of code as well. You talk
 about how the boxplot can be re-colored but no boxplot is produced to show it.
 Please remove the 'eval = FALSE'.

- [ ] REQUIRED: Line 98, split this comment onto multiple lines. When the 
vignette is built, the comment is too long for the code box and gets cut off at
 "... is constructed only" 

- [ ] REQUIRED: There should be a final section 'Session Info' that includes 
`SessionInfo()`. 

# man pages #

*Input*

- [ ] REQUIRED: Where does the Big Papi data come from?

*counts*

- [ ] REQUIRED: Is 'Big Papi SynLet' a package that the data comes from? Is 
'Big Papi' the name of the data that comes from the 'SynLet' package? Please be
 more clear with where this data comes from.

*gemini_calculate_lfc*

- [ ] CLARIFICATION: Why is the last line of the example code not run? 

*gemini_parallelization*

- [ ] CLARIFICATION: Can you provide an example for this function since it is 
exported.

*guide.annotation*

- [ ] REQUIRED: Same with counts, please be more clear about where the data 
comes from.

*sample.replicate.annotation*

- [ ] REQUIRED: Same as with counts and guide.annotation, please be more clear 
about where the data comes from.

# R code #

- [ ] NOTE: Consider clarifying how 9 object(s) are initialized.
        function (object)
        - gemini_boxplot (.)
        - gemini_boxplot (.)
        - gemini_boxplot (gi)
        - gemini_boxplot (hj)
        - gemini_boxplot (label)
        - gemini_boxplot (y)
        - gemini_calculate_lfc (.)
        - update_s_pb (.)
        - update_tau_pb (.)

- [ ] SUGGESTION: For formatting purposes, consider shorter lines. 330 lines 
(9%) are > 80 characters long.

- [ ] SUGGETSION: For formatting purposes, consider 4 spaces instead of tabs. 
1528 lines (43%) contain tabs.

- [ ] SUGGESTION: For formatting purposes, consider mulitples of 4 spaces for 
line indents, 139 lines (4%) are not.

- [ ] REQUIRED: There are some functions that need to have their arguments 
spaced better. For example, in 'gemini_calculate_lfc' lines 38-41 need to be 
better in line with the function. This is done correctly in 'gemini_initialize' 
line 49-63. Please be sure these spacing issues are fixed.

Best,
Kayla
ssj1739 commented 5 years ago

DESCRIPTION

NAMESPACE

vignettes

BiocManager::install("gemini")



- [X] SUGGESTION: We strongly encourage the use of table of contents.

- [X] REQUIRED: Please include some text that explains where the Big Papi data
comes from.

 - [X] SUGGESTION: For the sections using knitr::kable(), I would change the
'echo = FALSE' to true so that the user can see how to achieve the nice table
you produce. Especially when you start subsetting by A549 (line 111, 117), the
user may not know how to do this and showing the code would make the code
reproducable.

 - [X] REQUIRED: Line 122, since 'eval = FALSE' the plot is not produced in the
vignette. So when you say "We can see that GEMINI..." we can't because the plot
is not there. Please remove the 'eval = FALSE'.

- [X]  REQUIRED: Line 137, the same goes for this chunk of code as well. You talk
about how the boxplot can be re-colored but no boxplot is produced to show it.
Please remove the 'eval = FALSE'.

- [X]  REQUIRED: Line 98, split this comment onto multiple lines. When the
vignette is built, the comment is too long for the code box and gets cut off at
"... is constructed only"

- [X] REQUIRED: There should be a final section 'Session Info' that includes
SessionInfo().

#man pages
**SJ: I clarified all data as coming from a previous publication (Najm et al, 2018 *Nature Biotechnology*) Details were added in the respective man pages.**

*Input*

- [X] REQUIRED: Where does the Big Papi data come from?
*counts*

- [X] REQUIRED: Is 'Big Papi SynLet' a package that the data comes from? Is
'Big Papi' the name of the data that comes from the 'SynLet' package? Please be
more clear with where this data comes from. **SJ: I clarified all data as coming from a previous publication (Najm et al, 2018 *Nature Biotechnology*) Details were added in the respective man pages.**
gemini_calculate_lfc

- [X] CLARIFICATION: Why is the last line of the example code not run?

gemini_parallelization

- [X] CLARIFICATION: Can you provide an example for this function since it is
exported. **SJ: Removed the \dontrun tag**
guide.annotation

- [X] REQUIRED: Same with counts, please be more clear about where the data
comes from. **SJ: I clarified all data as coming from a previous publication (Najm et al, 2018 *Nature Biotechnology*) Details were added in the respective man pages.**
sample.replicate.annotation

- [X] REQUIRED: Same as with counts and guide.annotation, please be more clear
about where the data comes from. **SJ: I clarified all data as coming from a previous publication (Najm et al, 2018 *Nature Biotechnology*) Details were added in the respective man pages.**

# R code
- []  NOTE: Consider clarifying how 9 object(s) are initialized.
function (object)
 **SJ: These result from variables used in tidyverse/ggplot2 environments, and are not global or exported.**
- gemini_boxplot (.)
- gemini_boxplot (.)
- gemini_boxplot (gi)
- gemini_boxplot (hj)
- gemini_boxplot (label)
- gemini_boxplot (y)
- gemini_calculate_lfc (.)
- update_s_pb (.)
- update_tau_pb (.)

- [] SUGGESTION: For formatting purposes, consider shorter lines. 330 lines
(9%) are > 80 characters long.
**SJ: These are primarily in the documentation, and do not affect code readability.**

- [X] SUGGETSION: For formatting purposes, consider 4 spaces instead of tabs.
1528 lines (43%) contain tabs.

- [X] SUGGESTION: For formatting purposes, consider mulitples of 4 spaces for
line indents, 139 lines (4%) are not.

- [X] REQUIRED: There are some functions that need to have their arguments
spaced better. For example, in 'gemini_calculate_lfc' lines 38-41 need to be
better in line with the function. This is done correctly in 'gemini_initialize'
line 49-63. Please be sure these spacing issues are fixed.
**SJ: All lines were reformatted using RStudio reformatting specifications**
bioc-issue-bot commented 5 years ago

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

8917d89 Increment to version 0.99.9. Added documentation t...

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

b8e38aa Added tests 53694eb Increment to 0.99.10

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:

e27f73b Increment to 0.99.11

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

Hi @ssj1739,

Thank you for making the changes, everything looks good and I'm happy to accept the package!

Best, Kayla

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

https://bioconductor.org/packages/gemini

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.