Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

pmp #1268

Closed andzajan closed 4 years ago

andzajan commented 4 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 4 years ago

Hi @andzajan

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: pmp
Type: Package
Title: Peak matrix processing
Version: 0.2.5
Author: Ralf Weber and Andris Jankevics
Maintainer: Andris Jankevics <a.jankevics@bham.ac.uk>
Description: Tools and filters for peak matrix scaling, normalisation and filtering.
License: GPL-3
biocViews: MassSpectrometry, Metabolomics, Software
Depends: R (>= 3.6)
Imports: stats, impute, pcaMethods, missForest
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1
Suggests: 
    testthat,
    covr,
    knitr,
    rmarkdown,
    BiocStyle
VignetteBuilder: knitr

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.

andzajan commented 4 years ago

Note that the pmp packages is a dependency for the structToolbox (#1266).

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

26 November, 2019

I'm very sorry that your package did not get added to the review queue.

Please update this package in response to the following comments and to the automatic package build reports. When this package is acceptable, we will add your second contribution as an 'AdditionalPackage', as outlined in https://github.com/Bioconductor/Contributions#submitting-related-packages

DESCRIPTON

vignette

R / man (specific comments should be considered generally in your code)

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

andzajan commented 4 years ago

Thank you for your comments, they are very helpful. We are willing to follow your suggestions but as some of it requires quite significant changes it will take a bit of time, so please bear with us.

bioc-issue-bot commented 4 years ago

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

085029b Refinements for glog function bff2227 Added function and tests to plot lambda optimisati... 5296d22 ggplot2 dependency added e6016d3 Fixed namespace referencing inside pmp package its... ef05efd Glog lambda optimisation plot output added to the ... 2aac76f Prototype for S4 class and 'set' methods of glog t... 1b91165 Fixed that GlogOutput class can include ggplot obj... 2dbfa73 Fixed warnings/notes from BiocChek function. 29ce240 Added 'get' methods for 'GlogOutput' class 72fa4e1 Code style fixes e27d738 Merge branch 'master' into devel bfef142 glog transformation supports input of fixed lambda 2e89382 Redundant glog tests removed 209510c glog lambda optimisation plot improved code and fi... e887f71 Method names for GlogOutput class follow consisten... c9caff5 Namespace and description update for new version o... 18cb812 Packages version change, R version dependency chan... cb29d49 Removed S4 classes and methods for glog scaling fu... 38c2432 New internal functions to support 'SummarisedExper... f2ea721 rownames and colnames slots were not handled prope... d4e30fa missing function imports added 6318010 Glog scaling function prototype using SummarizedEx... 6c44435 check_input_data was extracting column names befor... 17c56fc Trying to fix AppVeyor build 1e42637 2nd attempt to fix AppVeyor build. 08ae753 glog_transformation will return the same data type... fae5e02 Non standard R data structures like DataFrame shou... 08757fc Function to plot glog lambda optimisation is indep... 910b536 Data for R function tests moved to non-exported sy... 2fbbc74 Full MTBLS79 data set added to the package data fo... 6cbb9e8 Refactoring of filter_peaks_by_blank.Rd Uses Sum... [fe04a3c](https://github.com/computational-metabolomics/pmp/commit/fe04a3c6f716a38db122131c6df301dc4e10ecdc) check_peak_matrix test added Check if function fa... [3dfa780](https://github.com/computational-metabolomics/pmp/commit/3dfa780fa8a5e6b33aca8bd5683adf8fd46143e9) Simplified logical expression [133896f](https://github.com/computational-metabolomics/pmp/commit/133896fcaf5dade3dd547dd9eaa864799ef85533) Clarified logical expression. [cf73345](https://github.com/computational-metabolomics/pmp/commit/cf7334545858d26530d24f4fd620db14e22bbb09) Checking for data type is not needed incheck_pea... afaf59e Simplified code to create 'flags' meta data. 48b6dcd Fixed devtools:check() failing on tests. c719cb9 SummarizedExperiment object support added to nor... c5ad570 Some improvement in input/output data structure ha... 60f208e If in SummarizedExperiment object rowData are ... c5ddbc4 T was used instead of TRUE in test function. ad760d8 Added function to list all used parameter values w... d004da1 Support for SummarizedExperiment in filter_sampl... 1f86d41 supports SummarizedExperiment input/output. 3fb3756 support for SummarizedExperiemnt object added re... e11ea31 'filter_samples_by_mv' supports SummarizedExperime... 4f0df84 Typo in 'filter_peaks_by_rsd' example. cf6905f function supports SummarizedExperiment objet. eadd1bf supports SummarizedExperiment object. 6a63688 For consistency across all functions parameter nam... 9aa40ed Removed empty lines from R scripts. 9b28636 Vectorised call is used insted of slower 'apply' f... 04501ea MV imputation with mean/median value of feature is... 483b3c2 Apply calls in normalisation functions replace wit... 2736e55 apply calls replaced with faster alternatives in p... 19524f7 Where possible apply calls replaced with faster al... 21a1d06 Temporarily Switching Travis/AppVeyor tu use R sta... 7da800d Temporarily Switching Travis/AppVeyor tu use R sta... 30cd341 The first round of documentation update. 5b9df3e AppVeyor failed to load Rccp.dll with stable R ver... 449b3a8 Help pages updated for glog transformation functio... 4a424a8 Support for SummarizedExperiment class in glog_pl... [149008c](https://github.com/computational-metabolomics/pmp/commit/149008c52ade57cf22706220db61591f1b5bde78) The 2nd round of documentation review. [4caa916](https://github.com/computational-metabolomics/pmp/commit/4caa916c5404adf9d34e37001f0e6644873facd0) Changed travis to use bioc-devel branch. [fa42fad](https://github.com/computational-metabolomics/pmp/commit/fa42fad3ac7445eba36848f959e9f3aac2a23122) Trying to fix Travis build. [23fafd4](https://github.com/computational-metabolomics/pmp/commit/23fafd4355ddcb3bafbe3778ffe0ff22c32c0a61) Updated DESCRIPTION and NEWS file. [bc27ef4](https://github.com/computational-metabolomics/pmp/commit/bc27ef4418421fd73b5ffd8fd3ab7681a9b2a3a3) Vignette draft. [01f92d9](https://github.com/computational-metabolomics/pmp/commit/01f92d98c04dd94b70cc28efd86eeb855dd3a767) Updates to the package vignette. [22f9570](https://github.com/computational-metabolomics/pmp/commit/22f95707982e2d5f201a1bdcc34c046e945beffd) Package vignette update. [bbcce67](https://github.com/computational-metabolomics/pmp/commit/bbcce67c9ef6b2b7eb044060a7990dc46b99de07) pqn coefficients excluded to be stored in processi... [3c832d9](https://github.com/computational-metabolomics/pmp/commit/3c832d97c289279ba7a621e97b3af618f0b37587) Updates to vignette structure. [4e350d1](https://github.com/computational-metabolomics/pmp/commit/4e350d187eaab5d6a9043acd4293e05626b0dcbb) QC-RSC batch correction method moved from separate... [d2896ac](https://github.com/computational-metabolomics/pmp/commit/d2896ac3e41e8745715220bf7b9442767769353f) Support forSummarizedExperimentclass added for... [d3fe7d1](https://github.com/computational-metabolomics/pmp/commit/d3fe7d141a1b721c9ac6d200ecf0ad3ee3d84bf0) Removed reference to sbcms Namespace [44bd32b](https://github.com/computational-metabolomics/pmp/commit/44bd32be78e0c2364ffd21474ab032e1d4b9c80a) Function to plot s/b correction outputs supports... ea2bf09 Documentation update for signal batch correction f... e2f8321 Add dummy class label for mv imputation if check_d... c4d25d7 Advanced vignette for signal batch correction algo... e14ff27 Final updates on signal batch correction vignettes... d83e2c4 Added instructions to open MTBLS79 help page. 6c08575 Prevent filter_peaks_by_fraction to over-write p... cdf2c24 Added generl vignette for pmp processing. 5ec1d40 Formatting 067e2bf Function to display processing history. c63b148 Apply call replaced with faster alternative. 71fe19b Updated NEWS file. d28d4d5 In case input is not SummarizedExperiment, add out... f732931 Changed wording of the warning message. a3868c2 Fixed tests failing on 32-bit architecture. 8f78ad3 BiocStyle required magick dependency now. cd5e944 Travis fix to install libmagick++-dev 35070cc Vignette was using list style output syntax. c471c43 Clear AppVeyor cache. f4e4f61 Merge pull request #21 from computational-metabolo...

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

andzajan commented 4 years ago

21 January, 2020

I'm very sorry that your package did not get added to the review queue.

Please update this package in response to the following comments and to the automatic package build reports. When this package is acceptable, we will add your second contribution as an 'AdditionalPackage', as outlined in https://github.com/Bioconductor/Contributions#submitting-related-packages

Dear Martin, thank you for comprehensive and helpful review. Please see our response below where we did try to address all comments and suggestions.

DESCRIPTION

  • [x] please provide a more informative Title and Description, so that users will be able to distinguish your package from the others on Bioconductor

DESCRIPTION file has been updated.

Vignette

  • [x] Please provide a vignette outlining how your package is used. This should not be a recapitulation of 'help' pages, but rather walk the user through the data input and analysis steps and scientific insights your package enables. In particular connect your contribution to other opportunities within the Bioconductor ecosystem, illustrating how the inputs and outputs to your package are relevant to other steps in a typical work flow.

Three vignette documents have been added: Peak Matrix Processing for metabolomics data sets; Signal and batch correction for mass spectrometry; Signal and batch correction, data assesment and correction.

R / man (specific comments should be considered generally in your code)

  • [x] The 'features in rows / samples in columns' peak matrix validated by check_peak_matrix() seems like a SummarizedExperiment. Ensure that your functions (all, e.g., filter_peaks_by_blank() work with such standard Bioconductor objects, e.g., by writing an S4 generic with methods for SummarizedExperiment and for a matrix.

All exported functions of pmp package are using SummarizedExperiment object as the main data structure. Instead of writing S4 generics we opted to provide several utility functions to handle checks on input/output structures.

  • [x] Write functions as endomorphisms that return the same object as the input. For instance filter_peaks_by_blank() should return a SummarizedExperiment, perhaps updating the metadata() to include flags, or if one wishes to be more ambitious creating a derived class that contains slots to contain flags.

As described above we have implemented several functions to handle input/output data structures. metaData(), colData() and rowData() functionality of S4Vecotrs and SummarizedExperiment are used to handle processing flags. Function return_function_args is used to extract argument values of each function call to be be appended to metaData() slot of output data object as processing_history.

  • [x] In the roxygen tags, provide more comprehensive description of input parameters and return values. For instance, indicate the class (numeric()) of peak_data.

All documentation entries have been updated.

  • [x] Provide better documentation of the testData object, for instance describing the steps a user would take to go from the @source link to the object in your package. The object is incredibly complicated (36 components) and a decent documentation would describe each element. Perhaps you would rather limit the data to that which is essential for use in your package?

Object testData is only used for function unit tests with testthat, it has been moved to internal sysdata.Rda file and is not exported anymore. We have added full data set of the same data MTBLS79. Steps used to create corresponding SummarizedExperiment object are provided in help page of MTBLS79 object.

  • [x] glog_transformation.R:34 here and elsewhere use more efficient versions than apply() on a matrix, e.g., rowMeans() or functions from the matrixStats package.

Where possible all apply calls are replaced with faster alternatives. Relevant Github commits:

  • [x] mv_imputation.R:74 this is an iteration (calling the function as many times as there are rows in the data frame) that should be replaced with a vectorized call (calling functions once on the entire object), e.g., if (rowSums(is.na(df)) == nrow(df)) {...}

Fixed in 9b28636

We have moved all functions from previously submitted sbcms package to the function base of pmp. In our opinion QCRSC algorithm for signal batch effect correction for mass spectrometry data sets fits very well with general scope of pmp package. QCRSC and related functions are utilising "SummarizedExperiment" object and other pmp functions as demonstrated in corresponding vignettes: Signal and batch correction for mass spectrometry; Signal and batch correction, data assesment and correction.

bioc-issue-bot commented 4 years ago

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

de6b432 Fixed warnings during Rd. documentation generation...

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

andzajan commented 4 years ago

Dear @mtmorgan , I did fix Warnings on Windows build from yesterday commit. And it looks that today's build "ERROR" originates from your build system and is not related to the package itself. Do I need to take any further action?

* Checking for bioc-devel mailing list subscription...
    Maintainer is subscribed to bioc-devel.
* Checking for support site registration...
Error in if (suppressMessages(content(response))) { : 
  argument is not interpretable as logical
Calls: <Anonymous> -> BiocCheck -> checkForSupportSiteRegistration
Execution halted
mtmorgan commented 4 years ago

thanks for the changes; no need to address today's problem. I'll take a detailed look at your revisions later today.

bioc-issue-bot commented 4 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 4 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/andzajan.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("pmp"). The package 'landing page' will be created at

https://bioconductor.org/packages/pmp

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.