AshKernow commented 6 years ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

Repository: https://github.com/crukci-bioinformatics/qPLEXanalyzer

Confirm the following by editing each check box to '[x]'

[x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.
[x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.
[x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.
[x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.
[x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

I am familiar with the essential aspects of Bioconductor software management, including:

[x] The 'devel' branch for new packages and features.
[x] The stable 'release' branch, made available every six months, for bug fixes.
[x] Bioconductor version control using Git (optionally via GitHub).

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 6 years ago

Hi @AshKernow

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: qPLEXanalyzer
Type: Package
Title: Tools for qPLEX-RIME data analysis
Version: 0.99.0
Date: 2018-04-23
Author: Matthew Eldridge, Kamal Kishore, Ashley Sawle
Maintainer: Ashley Sawle <ads2202cu@gmail.com>
Description: Tools for quantitative proteomics data analysis generated from qPLEX-RIME method.
License: GPL-2
LazyLoad: yes
Imports: tidyr, preprocessCore, limma, ggplot2, RColorBrewer, stats, utils,
         Biostrings, GenomicRanges, IRanges, graphics, BiocGenerics,
           purrr, tibble, ggdendro, grDevices, dplyr, magrittr
Depends: R (>= 3.4), Biobase, statmod, MSnbase  
Suggests: knitr
VignetteBuilder: knitr
biocViews: Proteomics, MassSpectrometry, Normalization, Preprocessing, QualityControl, DataImport

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

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

ff9a95a Updated R version dependency be792a2 Resaved RData files with compression xz

bioc-issue-bot commented 6 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.

LiNk-NY commented 6 years ago

Hi @AshKernow ,

Thank you for your submission. I will be reviewing your package in the coming days. Please reach out to me here if you have any questions. Don't forget to use the @LiNk-NY tag to notify me. Thanks!

Best regards, Marcel

LiNk-NY commented 6 years ago

Hi @AshKernow,

Please see the review below. Don't hesitate to contact me via the issue here.

Best regards, Marcel

qPLEXanalyzer #735

Please format code so that it fits within 80 columns.
Your repository is at ~40 Mb. Please reduce the repository size (.git) using the BFG repo cleaner.

DESCRIPTION

Looks good.
Please add a BugReports field to link to the issues page on GitHub
Remove LazyLoad field, it's no longer supported and is ignored.
Consider using Authors@R field instead of the Author and Maintainer fields

NAMESPACE

Names look good.
You might want to consider renaming convertToMSnset to something like dataframeToMSnset.
Where possible, reduce function name length.

R

You might want to write a function that will perform all the checks to the inputs of convertToMSnset and other functions.
Avoid the copy and append method in groupScaling, use the allocate and fill method.
Avoid 1:ncol(x) sequence generation, use the more robust seq_len and seq_along
There's no need to use '%in%'(x, TRUE) & ..., instead use x & ...
Consider opening up the corrPlot interface to allow users to select colors

Minor:

length(x) == 0 is equivalent to !length(x)

vignettes

Again, format the vignette to max 80 column width. It is easier to read and maintain.
Consider using gridExtra for displaying the figures (p1, p2, etc.)
Update or delete commented code if it is not running

AshKernow commented 6 years ago

Hi @LiNk-NY

Thanks very much for your helpful feedback. We will address the issues you've raised.

cheers

Ash

LiNk-NY commented 6 years ago

Please bump your package's version for another build in the BBS/SPB. Thanks, Marcel

AshKernow commented 6 years ago

Hi Marcel Will do, but we have a couple more fixes we want to do first. Hopefully in the next day or so. cheers Ash

LiNk-NY commented 6 years ago

Hi Ash, @AshKernow

Thank you for following up on the comments. Please submit your changes as soon as possible so to avoid closing the issue due to inactivity. Thank you.

Marcel

bioc-issue-bot commented 6 years ago

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

0f6931f Version bump to trigger rebuild

AshKernow commented 6 years ago

Version bump to avoid inactivity. Still need to deal with:

Your repository is at ~40 Mb. Please reduce the repository size (.git) using the BFG repo cleaner.
You might want to write a function that will perform all the checks to the inputs of convertToMSnset and other functions.

bioc-issue-bot commented 6 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 6 years ago

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

d15072c Version bump

bioc-issue-bot commented 6 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.

LiNk-NY commented 6 years ago

Hi Ash, @AshKernow Can you provide some updates on the status of the changes? Thanks, Marcel

bioc-issue-bot commented 6 years ago

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

c589a00 Updates to checking functions for contrasts and sc... cda3343 Update calling of checking functions ab34046 Added assertthat library 45d7d08 minor code style updates

bioc-issue-bot commented 6 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 6 years ago

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

441e8b7 Minor code style updates b111111 Version bump

bioc-issue-bot commented 6 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.

AshKernow commented 6 years ago

AdditionalPackage: https://github.com/crukci-bioinformatics/qPLEXdata

AshKernow commented 6 years ago

Hi @LiNk-NY

I have pushed the latest build. I have tried to address all of your original comments. There are still warnings as the packages is slightly over the 4M build size, this is due to the data, however, once the package is accepted we will strip the data out and use the associated data package (see additional package comment above).

cheers

Ashley

LiNk-NY commented 6 years ago

Hi Ashley,

It looks like you'd like to submit an accompanying data package (although the link is broken).

Stripping the data out after submission does not look favorable because it will result in a repository with a large .git/ folder.

It would be better to resubmit both packages as a new issue. In the meantime, I can close the issue.

Best regards, Marcel

AshKernow commented 6 years ago

Hi @LiNk-NY

I've fixed the data package link - the repository was private, I've switched it to public now.

The problem is that the vignette for the data package describes 9 experiments, for which we include data, and their analysis. It is therefore dependent on the qPLEXanalyzer package and so cannot be submitted first, as recommended by the section on "Submitting related packages". Likewise the qPLEXanalyzer vignette requires the data, but we can't submit the pair with a circular dependency.

I will look at reducing the data size for the qPLEXanalyzer package to the minimum required for the examples in its vignette, and then clean the repository again. Would that be okay?

cheers

Ash

LiNk-NY commented 6 years ago

Hi Ashley, @AshKernow

Please add the data package to the Suggests: field of qPLEXanalyzer. Then add qPLEXanalyzer to the Depends: field of the data package. This will avoid a circular dependency.

As far as the submission process goes, did you add an additional webhook for the data package? See here: https://github.com/Bioconductor/Contributions/blob/master/CONTRIBUTING.md#submitting-related-packages

The qPLEXanalyzer repository is just software and it should not be as big as it is currently. Please remove any extra data from the package and run the BFG repo cleaner to reduce the size of the repository.

Best regards, Marcel

bioc-issue-bot commented 6 years ago

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

099f693 Version bump

bioc-issue-bot commented 6 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 6 years ago

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

1343e9b Remove qPLEXdata and version bump

bioc-issue-bot commented 6 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.

AshKernow commented 6 years ago

Hi Marcel, @LiNk-NY

I have trimmed the data in the qPLEXanalyzer package and cleaned the repo with the BFG. I can't add the qPLEXdata into the DESCRIPTION under Suggests as this results in a build error:

checking package dependencies ... ERROR Package suggested but not available: 'qPLEXdata'

The webhook for the qPLEXdata package is in place.

cheers

Ash

LiNk-NY commented 6 years ago

Hi Ashley, @AshKernow

My apologies if I didn't make myself clear but I wanted you to keep qPLEXdata in the Suggest field of qPLEXanalyzer. Please do this first and include a version bump.

Once the build has completed:

Our SPB expert, Lori @lshep, suggested that you re-add the AdditionalPackage comment so that the SPB picks up and builds both packages.

Regards, Marcel

bioc-issue-bot commented 6 years ago

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

75e8f27 Re-added qPLEXdarta to suggests and version bump

bioc-issue-bot commented 6 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.

AshKernow commented 6 years ago

AdditionalPackage: https://github.com/crukci-bioinformatics/qPLEXdata

bioc-issue-bot commented 6 years ago

Hi @AshKernow,

Starting build on additional package https://github.com/crukci-bioinformatics/qPLEXdata.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your additional package repository will NOT trigger a new build.

The DESCRIPTION file of this additional package is:

Package: qPLEXdata
Type: Package
Title: Data accompanying qPLEXanalyzer package
Version: 0.99.1
Date: 2018-05-11
Authors@R: person("Kamal Kishore", "Developer",
              email = "kamal.fartiyal84@gmail.com",
              role = c("aut", "cre"))
Description: qPLEX-RIME and Full proteome TMT mass spectrometry datasets.
Depends: R (>= 3.5), qPLEXanalyzer
Imports: utils, knitr, MSnbase, dplyr
VignetteBuilder: knitr
License: GPL-2
biocViews: ExperimentData, MassSpectrometryData, Proteome
NeedsCompilation: no

bioc-issue-bot commented 6 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.

LiNk-NY commented 6 years ago

Hi Ashley, @AshKernow I will review the data package. In the meantime, please response to each of the items in the review.

Thank you for your attention.

Regards, Marcel

AshKernow commented 6 years ago

Hi Marcel, @LiNk-NY

Thanks.

With regard to your review of the qPLEXanalyzer package:

Please format code so that it fits within 80 columns - Done Your repository is at ~40 Mb. Please reduce the repository size (.git) using the BFG repo cleaner. - Done

DESCRIPTION

Looks good. Please add a BugReports field to link to the issues page on GitHub - Added Remove LazyLoad field, it's no longer supported and is ignored. - Removed Consider using Authors@R field instead of the Author and Maintainer fields - Done

NAMESPACE

Names look good. You might want to consider renaming convertToMSnset to something like dataframeToMSnset. - We do not wish to give the impression that the only possible input is a dataframe, we also support other formats e.g. tibble. Where possible, reduce function name length. - We have reduced these where we can

R

You might want to write a function that will perform all the checks to the inputs of convertToMSnset and other functions. - Done - see new checkArgs.R script and changes to the Allfunction.R script Avoid the copy and append method in groupScaling, use the allocate and fill method. - rewritten completely Avoid 1:ncol(x) sequence generation, use the more robust seq_len and seq_along - done There's no need to use '%in%'(x, TRUE) & ..., instead use x & ... - fixed this Consider opening up the corrPlot interface to allow users to select colors - added two new arguments to allow this

Minor:

length(x) == 0 is equivalent to !length(x) - Thanks for the tip, this trick passed me by!

vignettes

Again, format the vignette to max 80 column width. It is easier to read and maintain. - Done except for code chunk headers, I can't figure out how to shorten them Consider using gridExtra for displaying the figures (p1, p2, etc.) - done Update or delete commented code if it is not running - All code is now running

LiNk-NY commented 6 years ago

Hi Ashley, @AshKernow Thanks for the response!

We do not wish to give the impression that the only possible input is a dataframe, we also support other formats e.g. tibble.

Those familiar with tibbles know that data.frame methods work for that data class. You can also consider using coercion functions such as as(object, "MSnset").

I will have a final peek at the package before accepting it.

Regards, Marcel

LiNk-NY commented 6 years ago

Please update your package and bump the version number. Thanks.

bioc-issue-bot commented 6 years ago

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

dbece09 version bump

bioc-issue-bot commented 6 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 6 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.

AshKernow commented 6 years ago

Hi Marcel, @LiNk-NY

I rebuilt the package and we have no errors now.

cheers

Ash

LiNk-NY commented 6 years ago

Hi Ashley, @AshKernow Your package has been accepted. Thank you for contributing! Regards, Marcel

bioc-issue-bot commented 6 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 6 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/AshKernow.keys is not empty), then no further steps are required. Otherwise, do the following:

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 biocLite(\"qPLEXanalyzer\"). The package 'landing page' will be created at

https://bioconductor.org/packages/qPLEXanalyzer

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.

mtmorgan commented 6 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/AshKernow.keys is not empty), then no further steps are required. Otherwise, do the following:

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 biocLite(\"qPLEXdata\"). The package 'landing page' will be created at

https://bioconductor.org/packages/qPLEXdata

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.

Bioconductor / Contributions