Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

Qtlizer #1203

Closed matmu closed 5 years ago

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

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: Qtlizer
Title: ‚Qtlizer‘ for Bioconductor
Version: 0.99.0
Authors@R: c(person("Matthias", "Munz", role = c("aut", "cre"), 
      email = "m.munz@uni-luebeck.de"),
    person("Julia", "Remes", role = "aut")
   )
Description: Queries can be made to Qtlizer. The user receives a data frame with the response.
Depends: R (>= 3.6.0)
License: GPL-3
Encoding: UTF-8
LazyData: true
Imports: 
    httr
Suggests:  BiocStyle,
    testthat,
    knitr,
    rmarkdown
RoxygenNote: 6.1.1
VignetteBuilder: knitr
biocViews: GenomeWideAssociation, SNP, Genetics 
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.

mtmorgan commented 5 years ago

It seems that the return value should be a GenomicRanges::GRanges object, or perhaps VariantAnnotation:VRanges; please update your package to use existing Bioconductor infrastructure to increase code robustness and interoperability with other packages.

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:

ba1a8f8 Bumped version

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:

4f8c134 Updated .gitignore

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:

a7e11cb Removed non-ASCII characters from R file

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.

bioc-issue-bot commented 5 years ago

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

db46736 add parameters ld_method and corr as well as some ...

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 @matmu,

Before I can perform an initial review of the package there are a couple things that must be done.

  1. Please be sure you address @mtmorgan's comment:

It seems that the return value should be a GenomicRanges::GRanges object, or perhaps VariantAnnotation:VRanges; please update your package to use existing Bioconductor infrastructure to increase code robustness and interoperability with other packages.

  1. Please perform the required version bump.

Once both of these are done then I will be able to provide you a proper review of the package.

Best, Kayla

bioc-issue-bot commented 5 years ago

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

2c95f2e update version number

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.

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.

bioc-issue-bot commented 5 years ago

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

8076cdf Version bump + new import

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

b7fa313 Update DESCRIPTION

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

07b1294 Update DESCRIPTION

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: "skipped, TIMEOUT, 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:

41993e0 Update DESCRIPTION

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:

3532bc7 version bump

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.

matmu commented 5 years ago

Hello @matmu,

Before I can perform an initial review of the package there are a couple things that must be done.

  1. Please be sure you address @mtmorgan's comment:

It seems that the return value should be a GenomicRanges::GRanges object, or perhaps VariantAnnotation:VRanges; please update your package to use existing Bioconductor infrastructure to increase code robustness and interoperability with other packages.

  1. Please perform the required version bump.

Once both of these are done then I will be able to provide you a proper review of the package.

Best, Kayla

Dear Kayla,

we added the parameter return_obj which is "dataframe" by default, but can be set to "grange" in order to return a GenomicRanges::GRanges object. Also, the version has been bumped.

Best,\ Matthias

Kayla-Morrell commented 5 years ago

Hello @matmu,

Thank you for making those changes. Please see the initial review of the package below. The suggestion points are not mandatory but are strongly encourage. Comment back here with updates that have been made and when the package is ready for a re-review.

DESCRIPTION

Vignette

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

Man pages

Unit tests

R code

Best, Kayla

bioc-issue-bot commented 5 years ago

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

2f60665 version bump

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.

matmu commented 5 years ago

Hello @matmu,

Thank you for making those changes. Please see the initial review of the package below. The suggestion points are not mandatory but are strongly encourage. Comment back here with updates that have been made and when the package is ready for a re-review.

DESCRIPTION

  • [ ] SUGGESTION: Look at the 'Title:' field, the quotetation marks don't seem correct (please justify if this was intended).
  • [ ] REQUIRED: For the 'License:' field, you list a license that does not require '+ LICENSE'. Therefore the LICENSE file should be removed from the top level of the repository.
  • [ ] REQUIRED: 'LazyData:' should be set to false. Having this field set to true can slow down the loading of the package.
  • [ ] SUGGESTION: We strongly encourage the use of the 'BugReports:' tag that includes a relevant GitHub link for reporting issues. I know you have some links provided when the package is loaded, but this link would be located on the landing page on Bioconductor for the package.

Vignette

  • [ ] REQUIRED: Following the 'Introduction' section there should be an 'Installation' section that shows the user how to download the package from Bioconductor. This section can also include loading the package. The code should look something like this:
if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("Qtlizer")
  • [ ] SUGGESTION: For section 3 "Example function calls", you might mention what type of data should be read into the funtion. Is it only names of SNPs or can other data types be used? (I see later on in the example both a name of a SNP and a gene name are used when passing a vector)
  • [ ] REQUIRED: Line 47, look at the wording of the last sentence. I think it should read "...can be set with corr and ranges between 0 and 1.". There seems to be an extra 'between' before corr.
  • [ ] REQUIRED: Line 60, in the last sentence you only list one allowed value even though there is another one used in the example. Be sure to list both allowed values.

Man pages

  • [ ] SUGGESTION: Only functions that are exported need to have a man page. For those functions you have documented already that aren't exported, you can add the tage @keywords internal which will remove the function from the documentation index.

Unit tests

  • [ ] NOTE: Consider adding unit tests, we strongly encourage them.

R code

  • [ ] REQUIRED: Avoid 1:..., use seq_len() or seq_along() instead. Found in file 'get_qtls.R', line 186.
  • [ ] SUGGESTION: Thank you for incorporating the GRanges object in your code. Ideally we would have liked the default of 'return_obj' to be a GRanges object, but what you implemented works as well.
  • [ ] SUGGESTION: Look over our notes for querying web resources (http://bioconductor.org/developers/how-to/web-query/). You may find helpful information to be sure your package doesn't TIMEOUT when building each day on our build systems.

Best, Kayla

Dear Kayla,

we followed all your suggestions & requirements except the unit tests and pushed a new version.

Looking forward to hear from you!

Best, Matthias

Kayla-Morrell commented 5 years ago

@matmu - All the changes look good. I have just one more requirement and then I'll be happy to accept the package.

Vignette

Best, Kayla

bioc-issue-bot commented 5 years ago

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

b138597 Vignette

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.

matmu commented 5 years ago

@matmu - All the changes look good. I have just one more requirement and then I'll be happy to accept the package.

Vignette

  • [ ] REQUIRED: Thank you for including the 'Installation' section. This section of code can include eval = FALSE so that the warning messages aren't shown.

Best, Kayla

Done!

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

https://bioconductor.org/packages/Qtlizer

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.