Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

sSNAPPY #2599

Closed Wenjun-Liu closed 2 years ago

Wenjun-Liu commented 2 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 questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.

bioc-issue-bot commented 2 years ago

Hi @Wenjun-Liu

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: sSNAPPY
Title: Single Sample directioNAl Pathway Perturbation analYsis
Version: 0.99.0
Authors@R: 
    person("Wenjun", "Liu", , "wenjun.liu@adelaide.edu.au", role = c("aut", "cre"),
 comment = c(ORCID = "0000-0002-8185-3069"))
Description: A single sample pathway pertrubation testing methods that compute directional scores that can be used to test significance of pathway perturbation at individual-sample or collective treatment levels.  
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.2
LazyData: TRUE
Suggests: 
    BiocManager,
    BiocStyle,
    cowplot,
    DT,
    knitr,
    pander,
    rmarkdown,
    stringr,
    testthat (>= 3.0.0),
    tidyverse
Config/testthat/edition: 3
SystemRequirements: C++11
Imports: 
    dplyr,
    magrittr,
    rlang,
    stats,
    plyr,
    purrr,
    BiocParallel,
    graphite,
    Rcpp,
    tibble,
    ggplot2,
    ggraph,
    igraph,
    reshape2,
    org.Hs.eg.db
LinkingTo: 
    Rcpp,
    RcppArmadillo
Depends: 
    R (>= 4.1.0)
biocViews:
    Software, GeneExpression, GeneSetEnrichment
URL: https://wenjun-liu.github.io/sSNAPPY/
VignetteBuilder: knitr
vjcitn commented 2 years ago

Vignettes is taking inordinately long to compile. Can you thin it out? Maybe use some precomputed entities? If you need long tests, use that facility.

Wenjun-Liu commented 2 years ago

Hi Vince, Thank you for your suggestion! I have updated my vignette to let it use pre-computed results. Compiling of it should be fairly quick now. Please let me know if you see any other problems.

Cheers, Wenjun

bioc-issue-bot commented 2 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 2 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. 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Wenjun-Liu commented 2 years ago

Hi, I have made changes based on the build report but I cannot push to git@git.bioconductor.org:packages/sSNAPPY. When I tried to do it, it says "permission denied". I was trying to submit my SSH to the BioCredential site by firstly activating my account . But after filling in the information at: https://git.bioconductor.org/BiocCredentials/account_activation/, I didn't receive any email. Did I do something wrong? Thank you in advance for your help.

lazappi commented 2 years ago

Hi @Wenjun-Liu. Someone from @Bioconductor/core should be able to help out with this. As a first suggestion please check that your email in the package DESCRIPTION file is the same as the one you used to sign up on the BiocCredentials site.

lshep commented 2 years ago

@Wenjun-Liu did you check your spam folder? Sometimes the verification email gets flagged as spam?

bioc-issue-bot commented 2 years ago

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

Wenjun-Liu commented 2 years ago

Thank you @lshep and @lazappi! The verification emails did go to my spam folder. I should have checked!

bioc-issue-bot commented 2 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lazappi commented 2 years ago

Hi @Wenjun-Liu

Thanks for submitting sSNAPPY :tada:! Below is my review of your package. Please reply here if anything is unclear or needs any further explanation.

What next?

Please address the comments as best as you can. When you are ready for me to check the package again please reply to let me know with a summary of changes you have made or any other responses.

Luke

Review

Key: :rotating_light: Required :warning: Recommended :green_circle: Optional :question: Question

General package development

DESCRIPTION file

NAMESPACE file

Package data

Documentation

README

```r
if (!requireNamespace("BiocManager", quietly=TRUE))
    install.packages("BiocManager")
BiocManager::install("sSNAPPY")


### Vignette

- [ ] :rotating_light: Please add a table of contents to the vignette
- [ ] :rotating_light: Please avoid having unevaluated chunks in the vignette
- [ ] :rotating_light: Please avoid having hidden chunks in the vignette (`eval = FALSE`)
- [ ] :rotating_light: Please add an *Installation* section to the vignette with the standard installation chunk (this should have `eval = FALSE`)
- [ ] :rotating_light: The vignette should not create new files (see the comment about caching below)
- [ ] :green_circle: Please see the **{BiocStyle}** for details on how to format the vignette (things like linking to other packages)
- [ ] :green_circle: I found some of the code in the vignette difficult to read. You may want to look at how it is formatted or even if some of it could be made into functions in the package.
- [ ] :warning: The output of the `ssPertScore` chunk was empty when I ran the vignette, please check what is happening here

## Code

### R

- [ ] :rotating_light: Please use `vapply` instead of `sapply`
- [ ] :rotating_light: Please use `seq_along()`/`seq_len()` instead of `1:...`
- [ ] :rotating_light: No not need to use `paste0` in `message()`, `warning()`, `stop()`
- [ ] :green_circle: Please think about whether you could support a standard Bioconductor object like `SummarizedExperiment`
- [ ] :warning: Consider adding validity checks for function arguments in exported functions
- [ ] :rotating_light: Please check you are following the guidelines for [Querying Web Resources](http://bioconductor.org/developers/how-to/web-query/)
- [ ] :rotating_light: Downloaded files should be cached with `BiocFileCache`. This will make things easier for users and avoid some of the issues you have with files being saved.
- [ ] :warning: It would be better if `weightedAdjMatrix()` returned the object rather than just creating a file which the user needs to load
- [ ] :rotating_light: Please avoid `suppressWarnings()` if possible

### C and Fortran code

- [ ] :rotating_light: Please check that your C code checks for user interruption
- [ ] :rotating_light: Please removed the commented code in the C source files

### Third-party code

- [ ] :rotating_light: I noticed some code borrowed from other packages. Please make sure this follows the [guidelines](https://contributions.bioconductor.org/third-party-code.html) and matches the original license.
bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Wenjun-Liu commented 2 years ago

Hi Luke @lazappi, thank you very much for your suggestions! I have fixed my codes accordingly. Please see the summary below:

General package development

lazappi commented 2 years ago
  • [x] ⚠️ Please address as many of the notes from BiocCheck::BiocCheck() as possible

There are still several notes in the build report. Please address those, particularly the following:

  • [x] 🚨 Please be consistent with the use of camelCase or snake_case for function names

There is still a mixture of styles used, for example compute_perturbationScore() should be either compute_perturbation_score() or computePerturbationScore()

  • [ ] 🚨 Please avoid having unevaluated chunks in the vignette
  • Some of my functions take very long to run so I saved the output of those functions in inst/extdata and set the chunks holding those code to eval = FALSE so my vignette won't take forever to build. Could you let me know if you have any alternative suggestions?

I don't think any of the chunks in your vignette took a long time to run. I would set everything to eval = TRUE and see if any of the checks complain. If they don't everything is fine, if they do you can consider other options like using a smaller example dataset or providing pre-computed version of some of the steps.

  • [ ] ⚠️ The output of the ssPertScore chunk was empty when I ran the vignette, please check what is happening here
  • I couldn't replicate this problem. Did codes following that chuck work alright for you?

This still happens with the current version and then there is an error in the normalizedScores chunk. It seems to be fine on the build system though so maybe it's something I did wrong.

  • [x] 🟒 Please think about whether you could support a standard Bioconductor object like SummarizedExperimen
  • The package still only accepts logCPM matrix at this stage but I will keep this at the back of my mind!

Please look at doing this before the package is accepted. It is not a lot of work and will make your package compatible with a lot of existing Bioconductor packages. All you have to do is write small wrappers or methods which extract the required information from a SummarizedExperiment and pass it to your current functions.

🚨 Downloaded files should be cached with BiocFileCache. This will make things easier for users and avoid some of the issues you have with files being saved. ⚠️ It would be better if weightedAdjMatrix() returned the object rather than just creating a file which the user needs to load

  • I have changed weightedAdjMatrix() (now called retrieve_topology()) to return the object directly, instead of storing the output as a file. This function is a wrapper around a chain of functions from the graphite package. I believe that they have dealt with the caching problem. But please let me know if there's anything that you think i should add to my package!

I just want to be sure how you have handled this. I think what you are doing now is downloading files to a temporary directory and then returning an object from the function? This would need to be done in every new R session. If you want to reuse things between sessions then you will need to use {BiocFileCache}. This is a little bit trickier because {graphite} does the downloading but might be worth it if what you are downloading doesn't change often.

  • [x] 🚨 Please avoid suppressWarnings() if possible

This is still used in compute_perturbationScore()

Third-party code

  • [x] 🚨 I noticed some code borrowed from other packages. Please make sure this follows the guidelines and matches the original license.
  • Codes for one of the helper functions, which replaces the nth occurrence of a string with a pattern was borrowed from a github package (martinctc/textworks). The original function prints a message for every search it does and it cannot be turnt off. I have changed the codes to turn that feature off.

The {textworks} package has a GPL-3 license which requires any package that copies, modifies or resues code to have the same license. Please make sure you have read this license, understand what the requirements are and have followed them.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 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, skipped". 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. 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 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. 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Wenjun-Liu commented 2 years ago

Sorry Luke @lazappi! I have fixed all problems that I overlooked now, except the caching one. To be honest with you, I am not very familiar with using {BiocFileCache}. I gave it a try but didn't have much luck. Is this something that I could possibly add on after my package is accepted?

Apart from that, I have fixed:

lazappi commented 2 years ago

Sorry Luke @lazappi! I have fixed all problems that I overlooked now, except the caching one. To be honest with you, I am not very familiar with using {BiocFileCache}. I gave it a try but didn't have much luck. Is this something that I could possibly add on after my package is accepted?

Apart from that, I have fixed:

  • [x] Please be consistent with the use of camelCase or snake_case for function names

The package should consistently use camelCase or snake_case but you still you have a mix of both (computePerturbationScore() and plot_gs_network(), should be with computePerturbationScore() and plotGSNetwork() or compute_perturbation_score() and plot_gs_network()).

  • [x] Please avoid having unevaluated chunks in the vignette
  • The permutedScore step in the vignette takes around 20 minutes to run on a 8-core machine. When I set that chuck to eval = TRUE, I got a timeout error. So now I set it back to eval = FALSE and provided pre-computed output. Hope that's ok! (I have reduced the example dataset size by half. But it still takes that long to run. If I use a even smaller subset of example data, I won't be able to have enough output demonstrating the use of visualisation function)

Ok, I have looked at this again and that chunk does take a long time to run. Maybe I accidentally skipped over it before, sorry for that. I think the solution you have now with showing that code but loading the results from a saved file is ok. Please echo the code that loads the data and include some text that explains what you are doing. There are some other code chunks where echo = FALSE, these should be shown to the user as well. The normalisedScores chunk also has eval = FALSE but that is quick so should be evaluated.

I also noticed the gsTopology.rda has returned to the vignettes/ directory. Please remove this from the git repository and check that it is not created by running the vignette.

  • [x] The output of the ssPertScore chunk was empty when I ran the vignette, please check what is happening here
  • I think I have found the problem behind this and fixed it. The error shouldn't occur anymore.

Yep, worked fine now πŸ‘πŸ» .

  • [x] Please think about whether you could support a standard Bioconductor object like SummarizedExperiment
  • SummarizedExperiment and DGEList are now accepted as inputs.

Awesome, thanks for doing this! πŸŽ‰ It will make things much smoother for users.

I think there are just the small things with the function names and the vignette and then you will be good to go.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 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. 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 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/sSNAPPY to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Wenjun-Liu commented 2 years ago

Thank you for your prompt response @lazappi! I have fixed the naming problem, removed the gsTopology.rda from vignette/ directory, and set all vignette chunks to be echo = TRUE. For the normalisedScores chunk, I set it to be eval = FALSE because output of the permutedScore step was too big so I only stored a subset of it but with the subset of permutation result I still wouldn't be able to show the use of visualisation function. So I also pre-computed the full normalisedScores. I realised it could be confusing so I removed the subsetted permutation result and have added texts to explain that the permutedScore and normalisedScores steps were set to be not executed because they take long time to run and the final result was pre-computed. Hope it makes sense!

lazappi commented 2 years ago

I think this is ok now, I will approve the package. Congratulations on getting it into Bioconductor πŸŽ‰! It can take a couple of days to be picked up by the build system but should hopefully make it into the next release.

bioc-issue-bot commented 2 years ago

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

Thank you for contributing to Bioconductor!

Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.

Wenjun-Liu commented 2 years ago

Thank you @lazappi! This is great news! I really appreciated all of your help and feedback. Looking forward to the new bioconductor release. :)

lshep commented 2 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/Wenjun-Liu.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("sSNAPPY"). The package 'landing page' will be created at

https://bioconductor.org/packages/sSNAPPY

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.