Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

MOMA #1462

Closed sunnyjones5 closed 4 years ago

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

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: moma.gbmexample
Type: Package
Title: Data Package For Moma Vignette
Version: 0.99.0
Author: Sunny Jones
Maintainer: Sunny Jones <sunnyjjones@gmail.com>
Description: This package provides the data used in the vignette of the moma package. 
Depends: R (>= 4.0.0)
License: GPL-3
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.1.0
biocViews: Software, NetworkEnrichment, NetworkInference, Network, FeatureExtraction, Clustering, FunctionalGenomics, Transcriptomics, SystemsBiology

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.

sunnyjones5 commented 4 years ago

Hi Reviewers,

This is just the data package for the vignette of the package 'MOMA' that I am also submitting. As per the directions on the contributions page I am not adding it until I receive the "review in progress" notification, but I just wanted to clarify.

Thanks, Sunny

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.

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.

bioc-issue-bot commented 4 years ago

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

fcce5bd fixing errors for biocheck build report - added Rp...

bioc-issue-bot commented 4 years ago

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

559b442 removed Rproj file from cache so that it won't con...

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.

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.

sunnyjones5 commented 4 years ago

Hi the only error I still have left is that the package is too large (5.9 MB) but it's only a data package and I've tried to compress it as much as possible. Please let me know how to proceed.

sunnyjones5 commented 4 years ago

This is the official package I'm submitting

~AdditionalPackage: https://github.com/califano-lab/MOMA~

bioc-issue-bot commented 4 years ago

Hi @sunnyjones5,

Starting build on additional package https://github.com/califano-lab/MOMA.

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: MOMA
Title: Multi Omic Master Regulator Analysis
Version: 0.99.0
Authors@R: c(person("Evan", "Paull", email = "evanpaull@gmail.com", role = c("aut")), 
          person("Sunny", "Jones", email = "sunnyjjones@gmail.com", role = c("aut", "cre")), 
          person("Mariano", "Alvarez", email = "reef103@gmail.com", role = c("aut")))
Description: This package implements the inference of candidate master regulator proteins from multi-omics' data (MOMA) algorithm, 
  as well as ancillary analysis and visualization functions. 
Depends: R (>= 4.0.0)
License: GPL-3
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.1.0
biocViews: Software, NetworkEnrichment, NetworkInference, Network, FeatureExtraction, Clustering, FunctionalGenomics, Transcriptomics, SystemsBiology
Imports:
    circlize (>= 0.4.8),
    cluster (>= 2.0.0),
    ComplexHeatmap (>= 2.2.0),
    dplyr (>= 0.8.0),
    ggplot2 (>= 3.2.0),
    graphics (>= 3.5.0),
    grid (>= 3.6.2),
    grDevices (>= 3.5.0),
    magrittr (>= 1.5.0),
    methods (>= 3.5.0),
    MKmisc (>= 1.1),
    moma.gbmexample (>= 0.9.0),
    parallel (>= 3.5.0),
    qvalue (>= 2.12.0),
    RColorBrewer (>= 1.0-5),
    readr (>= 1.3.1),
    reshape2 (>= 1.4.3),
    rlang (>= 0.4.0),
    stats (>= 3.5.0),
    stringr (>= 1.4.0),
    survival (>= 2.40-1),
    tibble (>= 2.1.1),
    tidyr (>= 1.0.0),
    utils (>= 3.5.0)
Suggests: 
BiocStyle, 
knitr,
rmarkdown,
testthat (>= 2.1.0)
VignetteBuilder: knitr
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.

bioc-issue-bot commented 4 years ago

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

247da0a fixes to errors being caused by data now being in ...

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.

bioc-issue-bot commented 4 years ago

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

c8c8e82 fixed R dependency warning

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: "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 4 years ago

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

1522120 fixed R dependency and changed biocviews to be an ...

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.

bioc-issue-bot commented 4 years ago

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

e86c8cd didn't add version change to trigger last bump

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.

bioc-issue-bot commented 4 years ago

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

96c1fc8 changed R dependency back to 4.0.0

bioc-issue-bot commented 4 years ago

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

f6fed2a changed R dependency back to 4.0.

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.

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.

bioc-issue-bot commented 4 years ago

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

7a64f18 changed R dependency and re-pushing to get rid of ...

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.

sunnyjones5 commented 4 years ago

The only remaining warnings are for the R dependency requirement which I need because both packages have compressed files in them.

Best, Sunny

LiNk-NY commented 4 years ago

Hi Sunny Jones, @sunnyjones5

Thank you for your submission. I have taken a look at your package. Please see the review below. If you have any questions, feel free to post them here.

Best regards, Marcel


MOMA #1462

Thank you for your submission.

A few questions:

Is the moma.gbmexample package necessary? Does it contain previously unpublished data?

If the data is just for the examples in the vignette, perhaps you can take a subset of it so that you can provide a small representation in the MOMA vignette without the introduction of a second package.

Other than the dependency on ComplexHeatmap, the author does not make use of existing Bioconductor infrastructure.

DESCRIPTION

NAMESPACE

vignettes/

R/

build report

sunnyjones5 commented 4 years ago

Hi Marcel,

Thank you for your comments and review! I am currently communicating with the other builder of the package about the best way to move forwards with regards to your concerns about using the extra package for example data and best way to either restructure or shrink the size. But I did want to respond to confirm that I had seen your comments and that I am working on making the other improvements as per your concerns.

Also, more broadly to your question about it not directly interfacing with other Bioconductor infrastructure, while it doesn't directly use other data structures commonly used in Bioconductor we think the package would still be useful to other users in the community as the analysis is built on biology data frequently used by the Bioconductor community and in other packages (the original analysis coming out in a forthcoming paper used TCGA data but other multi-omic data could be used as well). Do you think it's still appropriate to to include this package in Bioconductor, or would other changes need to be made to make it a better fit?

Best, Sunny

LiNk-NY commented 4 years ago

Hi Sunny, @sunnyjones5

Is there infrastructure that you can use from viper to link to your package? It seems like viper matrices are required for your package though the dependency is not explicitly listed in the DESCRIPTION file. Also, consider using MultiAssayExperiment for multi-omic data type handling and management.

(Disclaimer: I am the maintainer of MultiAssayExperiment)

Thanks Lori @lshep for identifying a potential dependency for the package.

Best, Marcel

sunnyjones5 commented 4 years ago

Hi Marcel,

While output from viper is required for the analysis those results can be generated separately and via a few different ways which is why I didn't make it a direct dependency, but I can either make it a dependency or suggested package to make their relationship more clear. The main output used is just the sample vs protein matrix so there isn't much more object infrastructure overlap that I could see being made.

I will look more into the MultiAssayExperiment infrastructure in order to better understand how to make MOMA more compatible.

Thanks, Sunny

bioc-issue-bot commented 4 years ago

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

27af309 pushing first set of changes/improvements based on...

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.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

sunnyjones5 commented 4 years ago

Hi Marcel,

Just an update that I've gone through and made most of the fixes to the package that you mentioned in the Description/Namespace/Vignette/R section. A few remarks about the things I am still working on and your other questions:

  1. Size of the example dataset: currently creating a workable subset now and will remove the other package from submission. I will also improve the structure of the object.
  2. Internal functions: yes most of the functions are internal and the main ones are run as methods in the Moma object. I've gone through and added the internal tag throughout to make it more clear
  3. Constructor function: I'm not sure I understand your description of it as a "plain function that just filters input." Currently that's the main task of the function in that it checks that all the different inputs to the Moma object are in the correct format. I added some more comments/warnings in the code to make it more clear that that is what it was doing. Could you elaborate on this a bit more?
  4. MultiAssayExperiment: I am looking more into this to see if I can adapt the code to interface with this data object style
  5. Save method: I will implement a method in the Moma object that allows you to more flexibly save any of the outputs.

Thanks, Sunny

LiNk-NY commented 4 years ago

Hi Sunny, @sunnyjones5

I've updated the link to the repository in the initial comment and renamed the name of the issue. Thanks for working on a workable subset of data.

  1. Example constructor functions can be seen here:

https://github.com/Bioconductor/SummarizedExperiment/blob/81d41a110c1115df6ad5b144c40e6d171ab13c2f/R/SummarizedExperiment-class.R#L80-L105

https://github.com/Bioconductor/RaggedExperiment/blob/a3d17eae741f8c27adc601c46dce8b2aab4e987c/R/RaggedExperiment-class.R#L115-L154

They take the input and then filter / coerce / pre-process etc. before creating the target class.

  1. It doesn't have to be a save method. It can be called something else but essentially it takes the data in your class and puts it into a file.
bioc-issue-bot commented 4 years ago

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

1e48ced made gbm example data smaller

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.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

bioc-issue-bot commented 4 years ago

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

b2f3ef0 removed old links to removed data package in examp...

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.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

bioc-issue-bot commented 4 years ago

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

0410283 changed constructor function to use either MAE or ... 1c23db2 fixed lingering bugs related to new constructor fu... e98f5ed made updates to vignette to match new input of mae ec5fbf3 added saveData function to MOMA object c063750 add in saveData function. updated vignette 09599ce increased version number to trigger bioconductor p...

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.

bioc-issue-bot commented 4 years ago

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

12d6c89 trying rebuild to see if errors resolve

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.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

sunnyjones5 commented 4 years ago

Hi Marcel @LiNk-NY ,

I've made all the changes that you recommended. In particular:

  1. I made the example dataset a MultiAssayExperiment object and made the constructor function compatible with both MultiAssayExperiment objects as well as named lists.
  2. I cleaned up the constructor function to be more streamlined. I moved some of the check functions to their own separate functions to make the constructor itself shorter. I still kept many of the error/warning messages so it would be clear to people what happened if they did enter something incorrectly.
  3. I made a saveData() method in the Moma object where you can pass in any of the names of the results fields and save them as text files.

Let me know if you think I should do anything else to make the package ready for publication!

(Note: On this build report I had a series of weird errors on the tokay2 build that had to do with base packages not loading. I wasn't sure how to fix that and so I just resubmitted the package again a few minutes later and the build report didn't have any errors cause it looks like it didn't run on tokay2. I'm guessing that's because of something on Bioconductor's end but I figured I would clarify what happened).

Thanks Sunny

LiNk-NY commented 4 years ago

Hi Sunny, @sunnyjones5 Thank you for making those changes. The package looks better!

Minor:

Thank you!

bioc-issue-bot commented 4 years ago

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

fee9b59 changed input names to constructor. got rid of Bio... b83700b got rid of redundant error/warning messages 698c816 pushing new minor changes to bioconductor build

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.

bioc-issue-bot commented 4 years ago

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

98f8d30 re-added in BiocManager cause of vignette install ...

sunnyjones5 commented 4 years ago

Hi Marcel @LiNk-NY ,

Thanks for getting back to me! Regarding your remaining concerns:

  1. I thought I needed to import BiocManager to facilitate installation of it as a Bioconductor package and to have the install section in the vignette. Without having it imported I get a note/error about using it in the vignette so I'm not sure I can take it out or how to properly do so?
  2. I took out the remaining redundant error and warning text from messages.
  3. I changed the MomaConstructor inputs to allow that naming flexibility.
  4. Unfortunately for certain/most results data structures the lists are too nested for them to be written by writeLines or to be easily tidied. From my searching on this sink was the best option. Is there a reason I shouldn't use it? I could add in a note/warning that alerts people that it's the function that's going to be used? Alternatively I could have them save as RData files or some other format that maintains their list structure? Another option would be to go back to not having the saveData function and allow users to use the data as they see fit as the main outputs are the plots generated by the makeSaturationPlots function? I can send over pictures/or longer descriptions of the results data if that would help explain why attempting to tidy them would be pretty difficult.
  5. Is the on.exit thing you mentioned related to the print options I currently change in the saveData() method?

Thanks, Sunny

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.