Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

planet #1826

Closed wvictor14 closed 3 years ago

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

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: planet
Title: Placental DNA methylation analysis tools
Version: 0.99.0
Authors@R: 
    c(person("Victor", "Yuan", email = "victor.2wy@gmail.com",
  role = c("aut", "cre")),
    person(c("Wendy", "P."), "Robinson",
  role = "ctb"))
URL: 
    https://victor.rbind.io/planet, http://github.com/wvictor14/planet
BugReports: 
    http://github.com/wvictor14/planet/issues
Description: This package contains R functions to infer additional biological
    variables to supplemental DNA methylation analysis of placental data. This 
    includes inferring ethnicity/ancestry, gestational age, and cell composition
    from placental DNA methylation array (450k/850k) data. The package comes 
    with an example processed placental dataset.
Depends: 
    R (>= 4.0)
Imports: 
    methods,
    tibble,
    magrittr,
    dplyr
Suggests:
    ggplot2,
    testthat,
    tidyr,
    scales,
    EpiDISH,
    knitr,
    rmarkdown
License: GPL (>= 2)
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.1.1
VignetteBuilder: knitr
biocViews: Software, DifferentialMethylation, Epigenetics, Microarray, 
    MethylationArray, DNAMethylation, CpGIsland
Roxygen: list(markdown = TRUE)
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: "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/planet to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

wvictor14 commented 3 years ago

Hi, I keep receiving this error when trying to fetch/push to git@git.bioconductor.org:packages/planet

$ git fetch --all Fetching origin Fetching bioconductor git@git.bioconductor.org: Permission denied (publickey). fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists. error: Could not fetch bioconductor

I created new ssh keys and added it to github but still receive this error. I'm on windows.

$ git remote -v bioconductor git@git.bioconductor.org:packages/planet (fetch) bioconductor git@git.bioconductor.org:packages/planet (push) origin https://github.com/wvictor14/planet.git (fetch) origin https://github.com/wvictor14/planet.git (push)

wvictor14 commented 3 years ago

Hi, I got it working by manually adding my ssh-keys directly into the biocredentials app. I will push a build in a couple minutes that addresses the warning.

bioc-issue-bot commented 3 years ago

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

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

wvictor14 commented 3 years ago

@Kayla-Morrell the following warning is from the build report, I am not sure how to investigate the warning since I cannot reproduce on my local machine:

  • checking whether package 'planet' can be installed ... WARN WARNING Found the following significant warnings: Rd warning: cannot open file 'C:/Users/pkgbuild/packagebuilder/workers/jobs/1826/planet_20201221074502/planet.buildbin-libdir/00LOCK-planet/00new/planet/help/%>%.html': Invalid argument See 'C:/Users/pkgbuild/packagebuilder/workers/jobs/1826/planet_20201221074502/planet.Rcheck/00install.out' for details.

Could you advise how I should proceed?

It also says there is a log that I might be able to get more information, but I'm not sure how to access:

See 'C:/Users/pkgbuild/packagebuilder/workers/jobs/1826/planet_20201221074502/planet.Rcheck/00check.log'

Kayla-Morrell commented 3 years ago

@wvictor14 - We've been experiencing some issues with our windows builder. I'd be happy to review the package in the mean time and if the warning persists we can investigate further on our end.

wvictor14 commented 3 years ago

Sounds good! Thanks @Kayla-Morrell , I'll wait for your review.

Kayla-Morrell commented 3 years ago

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

Data

Vignette

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

Man pages

R code

Best, Kayla

bioc-issue-bot commented 3 years ago

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

wvictor14 commented 3 years ago

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

  • [x] REQUIRED: Licence 'GPL (>= 2)' is unknown. Licenses cannot restrict use. I believe just 'GPL-2' should suffice. Also, the LICENSE file is not needed in the top directory since this is a standard license.

I changed the license to "GPL-2"

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

I set LazyData to false. I tried my best to modify code accordingly, but I receive these notes in R cmd check:

 predictAge: no visible global function definition for 'data'
  predictAge: no visible binding for global variable 'ageCpGs'
  predictEthnicity: no visible global function definition for 'data'
  predictEthnicity: no visible binding for global variable
    'ethnicityCpGs'
  Undefined global functions or variables:
    ageCpGs data ethnicityCpGs
  Consider adding
    importFrom("utils", "data")
  to your NAMESPACE file.

What I did after setting lazydata to false:

I'm not sure if this was appropriate or the best approach. Please let me know if there is a better way, or a way to address the notes. Thanks

Data

  • [x ] REQUIRED: The 'data-raw' content should be moved to an 'inst/script/' directory to be sure that they are included with the package.

Done

  • [ ] REQUIRED: In the ethnicityCpGs.R file, the data/object should not be local. You should be demonstrating to the user how they can download/obtain the raw data and turn it into the right data object. I would suggest including any raw data in an 'inst/extdata/' directory.

I cannot include raw data that is used in this file. It is extremely massive (>100mb). I included this script because it simply documents how I obtained the necessary data used in the function predictEthnicity.

Vignette

  • [x] REQUIRED: Only vignette files (.Rnw or .Rmd) and any necessary static images should be in the vignette directory. All other files should be removed (for example, the .orig files should be removed).

Done.

  • [x] REQUIRED: Each vignette should have an installation section that explains to the user how to download/install the package from Bioconductor. The code should look something like the following and should inclue eval = FALSE.
if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("planet")
  • [x] SUGGESTION: We strongly encourage the use of table of contents.
  • [x] REQUIRED: Each vignette should end we a session information section and include sessionInfo().

Done

Man pages

  • [x] REQUIRED: The 'figures/' directory does not belong here. I would suggest moving it to and 'inst/figures/' directory.

Done

R code

  • [ ] REQUIRED: Normally we don't allow for sysdata.rda in packages. I was not able to locate where this file was used. Please remove if not needed.

The sysdata.rda object contains internal data that is used in the function predictEthnicity.

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

wvictor14 commented 3 years ago

@Kayla-Morrell Ready for re-review. Please see my previous comment where I respond to each of your points in-line. Let me know how to proceed.

I'm not sure what the Warning is from the build report means.

Thanks!

wvictor14 commented 3 years ago

@Kayla-Morrell I need to change the R version to >= 4.0 instead of >= 4.1, because otherwise my users cannot download the latest version of planet (or at least, it would be inconvenient for them to use). This produces a warning from BiocCheck, but I see that other package submissions have been flexible in the R version dependency. Is it OK if I use R (>= 4.0) from this point on?

Kayla-Morrell commented 3 years ago

@wvictor14 - Thank you for making the necessary changes. We can allow you to keep the R version at 4.0. Everything else looks good and 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!

wvictor14 commented 3 years ago

Thank you @Kayla-Morrell !

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

https://bioconductor.org/packages/planet

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.