Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

mixOmics #900

Closed mixOmicsTeam closed 6 years ago

mixOmicsTeam commented 6 years ago

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

mixOmics has been on CRAN since 2009 and we finally decided to convert to bioC. It is a big move for us and we are aware that our current submission needs further improvements. We hope to be able to stage some of our developments gradually. The future improvements we envisage in particular are to set up setGeneric and setMethods functions to fit in MultiAssayExperiments. We have not been successful so far and we need more time. We hope that mixOmics will benefit the bioC community and attract more users! You can see our engagement and commitment with the computational biology community at www.mixOmics.org

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

Please note the following:

  • R CMD check --as-cran gives 3 NOTES, all OK for bioconductor
  • R CMD BiocCheck gives:
    • 1 ERROR (remove package from CRAN as our package still needs to be live on CRAN
    • 1 WARNING: that we cannot address because it's relative to when we use the cpus argument in some of our functions, let me know if there is a work around this
    • 4 NOTES: those include unit tests and aesthetics of the code. Our package includes > 100 R functions and we are unsure how to implement the unit tests at this stage. We welcome any feedback from you.

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

Hi @mixOmicsTeam

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: mixOmics
Type: Package
Title: Omics Data Integration Project
Version: 6.4.0
Date: 2018-10-03
Depends: R (>= 3.5.0), MASS, lattice, ggplot2
Imports: igraph, rgl, ellipse, corpcor, RColorBrewer, parallel,
        dplyr, tidyr, reshape2, methods, matrixStats, rARPACK, gridExtra, grDevices, graphics, stats, utils
Suggests: BiocStyle, knitr, rmarkdown
Author: Kim-Anh Le Cao, Florian Rohart, Ignacio Gonzalez, Sebastien Dejean with key contributors Benoit Gautier, Francois Bartolo
        and contributions from Pierre Monget, Jeff Coquery, FangZou Yao, Benoit Liquet. 
Maintainer: Kim-Anh Le Cao <kimanh.lecao@unimelb.edu.au>
Description: Multivariate methods are well suited to large omics data sets where the number of variables (e.g. genes, proteins, metabolites) is much larger than the number of samples (patients, cells, mice). They have the appealing properties of reducing the dimension of the data by using instrumental variables (components), which are defined as combinations of all variables. Those components are then used to produce useful graphical outputs that enable better understanding of the relationships and correlation structures between the different data sets that are integrated. mixOmics offers a wide range of multivariate methods for the exploration and integration of biological datasets with a particular focus on variable selection. The package proposes several sparse multivariate models we have developed to identify the key variables that are highly correlated, and/or explain the biological outcome of interest. The data that can be analysed with mixOmics may come from high throughput sequencing technologies, such as omics data (transcriptomics, metabolomics, proteomics, metagenomics etc) but also beyond the realm of omics (e.g. spectral imaging). The methods implemented in mixOmics can also handle missing values without having to delete entire rows with missing data. A non exhaustive list of methods include variants of generalised Canonical Correlation Analysis, sparse Partial Least Squares and sparse Discriminant Analysis. Recently we implemented integrative methods to combine multiple data sets: N-integration with variants of Generalised Canonical Correlation Analysis and P-integration with variants of multi-group Partial Least Squares.
License: GPL (>= 2)
URL: http://www.mixOmics.org
BugReports: https://bitbucket.org/klecao/package-mixomics/issues
Repository: Bioconductor
VignetteBuilder: knitr
Date/Publication: 2017-02-06 12:08:22
Packaged: 2017-02-06 06:49:17 UTC; klecao
NeedsCompilation: no
LazyData: true
biocViews: Microarray, Sequencing, Metabolomics, Metagenomics, Proteomics,  GenePrediction, MultipleComparison, Classification, Regression
RoxygenNote: 6.0.1.9000

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.

mtmorgan commented 6 years ago

This package is on CRAN; when accepted in Bioconductor it must be removed from CRAN.

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, TIMEOUT, 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.

klecao commented 6 years ago

The biocheck we ran (v1.12) was ok but for some reason the V1.17 breaks down. We dont understand where the error is coming from, we suspect it is something with the vignette but not too sure what it is. Could you please advise as we are in the dark.

Error in vapply(vigdircontents, vigHelper, logical(1), builder = desc) : 
  values must be length 1,
 but FUN(X[[1]]) result is length 2
Calls: <Anonymous> ... BiocCheck -> checkVignetteDir -> checkVigBuilder -> vapply

Thank you

llrs commented 6 years ago

There is an error with the BiocCheck package (I opened a issue there). But also on your vignette, I copied a fragment from the beginning:

  %\VignetteIndexEntry{mixOmics}
  %\VignetteEngine{knitr::rmarkdown}
  %\VignetteEncoding{UTF-8}

bibliography: ["mybib.bib"]
biblio-style: apalike
link-citations: true

header-includes:
  - \usepackage{color}
---

<!--
  %% \VignetteEngine{knitr::knitr}
-->

It has two \VignetteEngine which are different, one is knitr::knitr and the other knitr::rmarkdown, there should be only one. I think that the second one with the <!-- --> is a mistake.

mixOmicsTeam commented 6 years ago

Hi bioC team, we would need advice on the following:

     * ERROR: Package Source tarball exceeds Bioconductor size
      requirement.
        Package Size: 6.5165 MB
        Size Requirement: 4.0000 MB

Our package includes many case studies as we try to give real examples for each method in our toolkit. Could you please advise how we could work around this?

The warnings from the check have been fixed and pushed.

llrs commented 6 years ago

To trigger another build you need to increment de version number.

To reduce the size of the package the usual route is to remove the vignette.html and to move the data to a specific data package linked to the software package.

mixOmicsTeam commented 6 years ago

We have incremented the version number.

We had already removed the vignette.html We would prefer at this stage not to create a specific software package for the data, and discuss with our reviewer directly @Liubuntu

Liubuntu commented 6 years ago

Hi @mixOmicsTeam ,

Thank you for your submission. But please remember to remove it from CRAN before we can accept it to Bioc. Also follow this link to add a web hook so that it could trigger a new build. A technical review will be give shortly. Thanks!

Cheers, Qian

llrs commented 6 years ago

Sorry if my comments weren't helpful. As a user of mixOmics I am excited to see the package coming to Bioconductor. I was only trying to help you with the process.

I want to offer my help regarding the unit tests. One good point to start with is conerting the examples into tests. I would recommend to add tests using testthat. If you want I could submit a pull request.

mixOmicsTeam commented 6 years ago

Hi Lluis,

Thanks for your support/help :) We already have all our examples as tests (and some more, https://github.com/mixOmicsTeam/mixOmics-CRAN/tree/master/running_scripts); we run them everytime to make sure the package is not broken, but it does not check for consistency between version, just that the code runs without errors. I'm not sure how to make tests using testthat (from what I saw the examples are quite simple) without saving a lot of mixOmics objects of the previous version and checking that the new versions give the same objects/outputs..

Best,

Dr. Florian Rohart Research Fellow - Applied Statistician Course coordinator

Institute for Molecular Bioscience The University of Queensland St Lucia, Brisbane, QLD 4072 http://florian.rohart.free.fr/ http://www.flickr.com/frohart/https://www.flickr.com/photos/119717703@N02/

On 10 Oct 2018, at 3:06 am, Lluís notifications@github.com<mailto:notifications@github.com> wrote:

Sorry if my comments weren't helpful. As a user of mixOmics I am excited to see the package coming to Bioconductor. I was only trying to help you with the process.

I want to offer my help regarding the unit tests. One good point to start with is conerting the examples into tests. I would recommend to add tests using testthat. If you want I could submit a pull request.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Bioconductor/Contributions/issues/900#issuecomment-428272642, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AlyvLS1-Mk30anVMusWZo5KtrrWten3wks5ujNehgaJpZM4XHN8b.

bioc-issue-bot commented 6 years ago

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

12c9800 Update DESCRIPTION

klecao commented 6 years ago

Hi @mixOmicsTeam ,

Thank you for your submission. But please remember to remove it from CRAN before we can accept it to Bioc. Also follow this link to add a web hook so that it could trigger a new build. A technical review will be give shortly. Thanks!

Cheers, Qian

Hi @Liubuntu thank you for your message. Of course once accepted on bioC we will deprecate mixOmics on CRAN. We just wanted to make sure this automated ERROR would not prevent a review. We have triggered 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, 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.

Liubuntu commented 6 years ago

Hi @mixOmicsTeam ,

I am looking at the package already and will talk with the other team members about the situation. For now, the ERROR would not prevent the review process.

Best, Qian

Liubuntu commented 6 years ago

Hi @mixOmicsTeam ,

Have talked with the Bioconductor core team, for the ERRORs,

  1. the tarball size. For the unzipped tarball, vignette.html is 6.3 MB and the data/ repo is 3.0MB. We are fine with the bigger size since you are having a very comprehensive vignette with images and data for case studies.

  2. the versioning. For new submission in Bioconductor, we are having the x.y.z versioning rules that usually 0.99.0 for initial submission, and when included, the "y" will be bumped to be odd number, which will be 1.0.0. In this specific situation, since mixOmics has been through a lot of versions on CRAN and has a good record of maintaining by including bug fixes and new feature, we would make exception here for the "x=6", but still the rule for bumping "y" will be done automatically. So two ways to go:

    • keep the current version of 6.4.2, then it will be 6.6.0 in the new release Bioc 3.8 and 6.7.0 in devel Bioc3.9.
    • you can also submit package using version of *"6.99."**, will will be 7.0.0 in new release of Bioc3.8 and 7.1.0 in devel Bioc3.9.
      Personally I like the 2nd for a major version bump as transitioning from CRAN to Bioconductor, but that's your call.
  3. Other than the support site, you do need to register on the mailing list as maintaining your packages.

Following is part of the technical review, please seek to address all issues. More might be added.

vignette

1. Download the package from Bioconductor.

{r getPackage, eval=FALSE}
if (!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
 BiocManager::install("pkgName")

 Or install the development version of the package from Github.

 {r, eval = FALSE}
  BiocManager::install(“githubUserName/pkgname”)

2. Load the package into R session

{r Load, message=FALSE}
 library(pkgName)

DESCRIPTION:

Best, Qian

Liubuntu commented 6 years ago

Continued:

NAMESPACE

R

  • Checking for library/require of mixOmics...
  • WARNING: The following files call library or require on mixOmics. This is not necessary. R/MCV.block.splsda.R, R/perf.R, R/tune.block.splsda.R, R/tune.spls.R, R/tune.splsda.R

Formatting:

man

mixOmicsTeam commented 6 years ago

Hi Qian,

Regarding this warning: it comes from the fact that we're using `parallel' to set up some parallel computing, and we send the mixOmics package to the remote clusters so that the code runs; hence some

   clusterEvalQ(cl, library(mixOmics))

I'm not too sure how we can avoid that properly. Do you have any suggestions?

Thanks

Best,

Dr. Florian Rohart Research Fellow - Applied Statistician Course coordinator

Institute for Molecular Bioscience The University of Queensland St Lucia, Brisbane, QLD 4072 http://florian.rohart.free.fr/ http://www.flickr.com/frohart/https://www.flickr.com/photos/119717703@N02/

On 12 Oct 2018, at 5:57 am, Qian Liu notifications@github.com<mailto:notifications@github.com> wrote:

Continued:

NAMESPACE

R

Formattinghttp://bioconductor.org/developers/package-guidelines/:

man

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Bioconductor/Contributions/issues/900#issuecomment-429097139, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AlyvLVH0vaofRJ_im_k6foW4HhSul27tks5uj6K9gaJpZM4XHN8b.

Liubuntu commented 6 years ago

Hi @mixOmicsTeam ,

I'm not expert in using clusterEvalQ... and didn't get any input until now. I understand that you are using parallel when passing the "ncpu" argument, But is it necessary to run the "library(mixOmics) for each of the cluster? In other words, do you want to try to remove this lineclusterEvalQ(cl, library(mixOmics)) from your scripts and see will the package work fine still? Thanks!

Best, Qian

frohart commented 6 years ago

Hi @Liubuntu , clusterEvalQ(cl, library(mixOmics)) is actually needed when using the package on HPC (it would work without it on a single machine, but not on a HPC). I guess we could try to replace clusterEvalQ(cl, library(mixOmics)) by passing every single functions of mixOmics used on the clusters, but that would make a mess compared to the simple library(mixOmics). Any good reasons why it's not good practice to do clusterEvalQ(cl, library(mixOmics)) ? Thanks

Liubuntu commented 6 years ago

Hi,

Thank you for your explaining on the difference in using HPC and single machine. If that is the case, we'll just pass it. The WARNING comes from BiocCheck, which checks for the keyword library()/require()... because in normal case, the package loading is not allowed and they should go to Depends/Imports/Suggests... in DESCRIPTION. As long as it's needed for functioning of this package, an exception could be made here.

Best, Qian

Liubuntu commented 6 years ago

Also please let me know if you have questions about the above review. Basically they are about coding styles for the R/ section. Those are based on the Bioconductor guidelines. While they would not directly affect the package acceptance, but are preferences that we would strongly encourage.

The issue about the vignette is important and must be addressed!

Thanks, Qian

bioc-issue-bot commented 6 years ago

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

c5aee5e 6.4.3

frohart commented 6 years ago

Hi @Liubuntu ,

We could update our scripts to follow more the bioconductor guidelines (re 1:, sapply, etc), but we do have more than 90 scripts so that would take a while to address all those comments (especially the 80 characters limits). Could we do a progressive change? Thanks. All the other comments have been taken into account (I believe)!

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

edf58db 6.4.4 53d5c70 6.4.4

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

b3e4996 update help files for check 369abaa rm sample.rda in datalist 695effe add testthat in suggests 13bcffd switch rgl from imports to suggests 766ebc9 update code and help files for rgl:: and dontrun o... a8e5537 6.4.5 test ok

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.

Liubuntu commented 6 years ago

Hi,

Thank you for making those changes and adding unit tests. Please correct the following:

vignette

The other looks good, and a progressive changes for the coding style sounds ok.

Best, Qian

Liubuntu commented 6 years ago

Hi @mixOmicsTeam ,

Seems that you have made the changes to vignette. But please make a version bump so that the daily build could catch that. As soon as you bump the version, we are good to go. Thanks!

Best, Qian

bioc-issue-bot commented 6 years ago

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

51747e2 version bumped

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.

Liubuntu commented 6 years ago

Hi @mixOmicsTeam ,

Sorry, one last thing to do. You need to remove mixOmics from CRAN before we could accept it to Bioconductor. Make a version bump after the removing so that the build report is clear. Thanks!

Best, Qian

mixOmicsTeam commented 6 years ago

@Liubuntu we have requested the archival of the R CRAN package and awaiting confirmation. We'll let you know when this is done. Thanks!

mixOmicsTeam commented 6 years ago

@Liubuntu see the following answer from the R CRAN team, could you please advise?

mixOmics does not seem to be established in BioC release yet. As long as that is not the case, it is advisable to double host it, e.g. with some version number on CRAN and a higher version number on BioC. Note that otherwise your reverse dependencies on CRAN

Reverse depends: bootsPLS, mixKernel, sgPLS, YuGene Reverse imports: MetabolomicsBasics, PCA4you, plsRcox, polyPK, RVAideMemoire Reverse suggests: PLSbiplot1

will break if mixOmics is no longer available from any repository for R release and older evrsions of R

.

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

https://bioconductor.org/packages/mixOmics

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.

mixOmicsTeam commented 6 years ago

Hi there, Just a quick follow up question: when we do modifications to the package, which branch of bioconductor should we push them on? master? RELEASE_3_8? If we do modifications on our own github, are they taking into account somehow or only the ones we push to the bioconductor github repo? Thanks!

llrs commented 6 years ago

See section "Where to commit changes" on this help page linked above. Usually the new developments go the development branch and bug fixes to the development branch (and to the RELEASE branch).

The modifications on the github repository aren't pulled into Bioconductor repository. See this help file about how to set them. Basically you'll need to set a remote repository to bioconductor.org and pull from there (to get the version bump performed by the Bioconductor team) and push to add the new changes to Bioconductor.