search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

fedup #1897

Closed rosscm closed 3 years ago

rosscm commented 3 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 3 years ago

Hi @rosscm

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: FEDUP
Title: Fisher's Test for Enrichment and Depletion of User-Defined Pathways
Version: 0.99.0
Date: 2021-02-12
Depends: R (>= 4.0)
Authors@R:
    person("Catherine", "Ross",
        email = "catherinem.ross@mail.utoronto.ca",
        role = c("aut", "cre"))
Description:
    An R package that tests for enrichment and depletion of user-defined
    pathways using a Fisher's exact test. The method is designed for versatile
    pathway annotation formats (eg. gmt, txt, xlsx) to allow the user to run
    pathway analysis on custom annotations. This package is also
    integrated with Cytoscape to provide network-based pathway visualization
    that enhances the interpretability of the results.
biocViews:
    GeneSetEnrichment,
    Pathways,
    NetworkEnrichment,
    Network
Imports:
    openxlsx,
    tibble,
    dplyr,
    data.table,
    ggplot2,
    ggthemes,
    forcats,
    RColorBrewer,
    RCy3,
    utils,
    stats
Suggests:
    testthat,
    knitr,
    rmarkdown,
    devtools,
    covr,
    rmdformats
SystemRequirements:
    Cytoscape (>= 3.8.2) (optional, but strongly recommended for visualization)
LazyData: true
Encoding: UTF-8
Language: en-US
License: MIT + file LICENSE
VignetteBuilder: knitr
URL: https://github.com/rosscm/FEDUP
RoxygenNote: 7.1.1
rosscm commented 3 years ago

I realize that I need to update the R version dependency to 4.1 to comply with Bioconductor 3.13. Can I change that now before the package gets reviewed?

mtmorgan commented 3 years ago

yes; during the 'moderation' stage your package will be cloned into the git.bioconductor.org repository and after that changes will need to be pushed there instead / as well, but until then you are free to modify your github repository exclusively.

rosscm commented 3 years ago

After updating the R version dependency to 4.1 my GHA workflows are failing, specifically R CMD check on macOS/ubuntu/windows releases (ubuntu devel works just fine as expected)

── R CMD build ─────────────────────────────────────────────────────────────────
* checking for file ‘.../DESCRIPTION’ ... OK
* preparing ‘FEDUP’:
* checking DESCRIPTION meta-information ... OK
* installing the package to build vignettes
      -----------------------------------
ERROR: this R is version 4.0.4, package 'FEDUP' requires R >= 4.1
      -----------------------------------

what should I do here?

mtmorgan commented 3 years ago

From Bioconductor's perspective, your package will never be available on R 4.0.4 (it'll be introduced to the devel branch of Bioc, which is on R-devel; it'll be included in the next and all subsequent releases, which will all be on R-4.1 or higher. It'll never be tested on R-4.0.4, or with packages from the 'Bioc 3.12' release. So from the Bioconductor perspective the failures on GHA for the release branch are irrelevant.

You could disable GHA on the release branch. You could maintain a separate 'Bioc' branch in your git repository with the version. You could revert the change to the R version dependency and live with the BiocCheck complaint, and tell your reviewer why you've done that (your reviewer is not likely to insist on the version change...)

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

rosscm commented 3 years ago

This issue seems to be happening since I updated my project name from FEDUP to fedup before the review processed started, so the Bioconductor git repo name is outdated. I bumped the version in my Description file to reflect that change and pushed to my local repo, but I'm unable to push to the Bioconductor git repo

git remote -v
origin  https://github.com/rosscm/fedup (fetch)
origin  https://github.com/rosscm/fedup (push)
upstream    git@git.bioconductor.org:packages/fedup (fetch)
upstream    git@git.bioconductor.org:packages/fedup (push)

git push origin main  
Total 0 (delta 0), reused 0 (delta 0)
To https://github.com/rosscm/fedup
   bb488a5..8dca7f1  main -> main

git push upstream main  
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 302 bytes | 302.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0)
remote: FATAL: W refs/heads/main packages/fedup rosscm DENIED by fallthru
remote: error: hook declined to update refs/heads/main
To git.bioconductor.org:packages/fedup
 ! [remote rejected] main -> main (hook declined)
error: failed to push some refs to 'git@git.bioconductor.org:packages/fedup'
lshep commented 3 years ago

The package has the correct case structure on git.bioconductor.org the issue I believe is you are trying to push to main. Bioconductor does not use main; it still uses master as the devel/default branch. Please push to git push upstream master

rosscm commented 3 years ago

This is what happens when I push to upstream master

git remote -v
origin  https://github.com/rosscm/fedup (fetch)
origin  https://github.com/rosscm/fedup (push)
upstream    git@git.bioconductor.org:packages/fedup (fetch)
upstream    git@git.bioconductor.org:packages/fedup (push)

git push upstream master
error: src refspec master does not match any
error: failed to push some refs to 'git@git.bioconductor.org:packages/fedup'

Looking at the bioconductor build report output

Package: FEDUP
--
Version: 0.0.0
RVersion: 4.1
BiocVersion: 3.13
BuildCommand:
BuildTime: NA
CheckCommand:
CheckTime:
BuildBinCommand:
BuildBinTime:
PackageFileSize: -1.00 KiB
BuildID:: FEDUP_20210223205547
PreProcessing: Starting Git clone.
PostProcessing: Git clone Failed.

the cloning of the repo itself is failing. Why is that happening?

lshep commented 3 years ago

The build report and the issues to pushing to git.bicoonductor.org are two unrelated issues.

The SPB uses a database to keep track of the issues. While you updated the repository name in github and DESCRIPTION, our internal database still had the uppercase. I have updated the database and it should no longer try to build with the uppercase version.

I can clone using the all lower case version 

git clone git@git.bioconductor.org:packages/fedup
Cloning into 'fedup'...
remote: Enumerating objects: 1090, done.
remote: Counting objects: 100% (1090/1090), done.
remote: Compressing objects: 100% (391/391), done.
remote: Total 1090 (delta 654), reused 1090 (delta 654), pack-reused 0
Receiving objects: 100% (1090/1090), 6.03 MiB | 5.63 MiB/s, done.
Resolving deltas: 100% (654/654), done.

Please try and git fetch --all before pushing to pull down the upstream branches.

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

Kayla-Morrell commented 3 years ago

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

DESCRIPTION

NEWS

Data

Vignette

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

Man pages

R code

Best, Kayla

rosscm commented 3 years ago

Thanks for your feedback, Kayla! I'll get back to you soon with my revisions.

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

rosscm commented 3 years ago

Hi Kayla,

The package is ready for a re-review. I'll outline the changes below:

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

DESCRIPTION

  • [ ] REQUIRED: 'LazyData' should be set to 'false'. It is our experience that when this field is 'true' it can slow down the loading of a package especially if it contains data.

Done.

  • [ ] REQUIRED: Update R version dependency from 4.0 to 4.1.

Done. I'm maintaining two branches: main (with R version 4.0 for local testing) and bioc (with R version 4.1 for bioc). Bioc is the branch being pushed to git@git.bioconductor.org.

  • [ ] REQUIRED: I would rethink listing Cytoscape as a system requirement. Do you use functionality from it in your code? Or do you just demonstrate using it in the README. I would think if it's just an option that you should remove it as a system requirement so users don't install because they think its a requirement in order to use your package. Then just keep it in the README as an option for better visualization.

Done.

  • [ ] SUGGESTION: We strongly encourage the use of the 'BugReports:' field that includes the relevant link to GitHub for reporting Issues.

Done.

NEWS

  • [ ] REQUIRED: It's great to include a NEWS file so your package will be included in our release annoucments. However, it doens't seem like your NEWS file is formatted properly. Please take a look at our documentation to be sure it is formated correctly and how you can check it on your own system. The documentation can be found here https://bioconductor.org/developers/package-guidelines/#news.

Done. I'm now using the NEWS.Rd format located in inst/ directory.

Data

  • [ ] REQUIRED: Since you include 'inst/extdata' in the package we required documentation for the data in an 'inst/script/' directory. The scripts in this directory can vary. Most importantly there needs to be documentation that clearly states 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 the data and roughly reproduce the file or object that is present as data.

Done.

  • [ ] REQUIRED: We don't allow the 'data-raw' directory in our packages. I would suggest moving these scripts to the 'inst/script' directory since they demonstrate how to obtain the 'data' files from the 'inst/extdata' files.

Done.

Vignette

  • [ ] REQUIRED: The Installation section should demonstrate to the user how to install and load the package from Bioconductor. The code should look like the following and include eval = FASLE.
if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("fedup")

Done.

  • [ ] REQUIRED: The only figures that should be in this directory are the ones that are used in the vignette. I only was able to find "fedupEM-1.png" used in the vignette. The others should be removed from here.

Done.

  • [ ] REQUIRED: The last section of the vignette should be 'Session Information' and include sessionInfo().

Done.

Man pages

  • [ ] REQUIRED: We don't allow for figure directories to be in the man page directory. If you need these figures for the README I would suggest moving them to an 'inst/figures/' directory.

Done.

  • [ ] REQUIRED: We need all exported functions to be tested in some way, whether that be runnable examples in the man pages, runnable examples in the vignette, or with unit testing. There is an issue that there is no testing being done for the 'plotFemap' function. I realize this function utilizes Cytoscape, but then maybe that would make a good candidate to actually have this installed on our builders so it could be tested. I have no knowledge of this program so I'm not sure if it would work. But we do need this function tested in some way to be sure that it works and catch when it's broken.

Done. I reached to the bioc-devel team about this, and they say there's no way for Cytoscape to be installed on the builders (since it requires a display to run, and the builders don't have displays attached). I instead added a tryCatch to return NULL on 'Cytoscape not running locally' error. So now the function can be tested to some capacity.

R code

  • [ ] SUGGESTION: For formatting purposes, consider multiples of 4 spaces for line indents. There are 48 lines that are not.

Done.

I also made some enhancements to the package that are outlined in inst/NEWS.Rd.

Best, Kayla

bioc-issue-bot commented 3 years ago

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

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

Kayla-Morrell commented 3 years ago

@rosscm - Thank you for making the necessary changes. I have looked them over and everything looks good. I'm more than happy to accept the package.

Best, Kayla

bioc-issue-bot commented 3 years ago

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

Thank you for contributing to Bioconductor!

rosscm commented 3 years ago

Thanks, @Kayla-Morrell!

vjcitn commented 3 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/rosscm.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("fedup"). The package 'landing page' will be created at

https://bioconductor.org/packages/fedup

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.