MOSim - Githubissues

Neurergus commented 5 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/Neurergus/MOSim

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

Hi @Neurergus

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: MOSim
Title: Multi-Omics Simulation (MOSim)
Version: 0.99.0
Authors@R: c(person("Carlos", "Martínez", email = "cmarmir+bioconductor@gmail.com", role = "cre"), person("Sonia", "Tarazona", email = "sotacam@gmail.com", role = "aut"))
Description: MOSim package simulates multi-omic experiments that mimic regulatory mechanisms within the cell, allowing flexible experimental design including time course and multiple groups. 
Encoding: UTF-8
Depends:
    R (>= 3.6)
License: GPL-3
LazyData: true
biocViews: Software, TimeCourse, ExperimentalDesign, RNASeq
Imports:
    HiddenMarkov,
    zoo,
    methods,
    matrixStats,
    dplyr,
    stringi,
    lazyeval,
    rlang,
    stats,
    purrr,
    scales,
    stringr,
    tibble,
    tidyr,
    ggplot2,
    Biobase,
    IRanges,
    S4Vectors
Suggests:
    testthat,
    knitr,
    rmarkdown,
    BiocStyle
Collate: 
    'AllClass.R'
    'AllGeneric.R'
    'Simulator.R'
    'SimulatorRegion.R'
    'ChIP-seq.R'
    'DNase-seq.R'
    'functions.R'
    'Simulation.R'
    'MOSim.R'
    'RNA-seq.R'
    'simulate_WGBS_functions.R'
    'methyl-seq.R'
    'miRNA-seq.R'
    'zzz.R'
RoxygenNote: 6.1.1
VignetteBuilder: knitr

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.

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:

213974f 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.

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:

5e698a6 added also aut field to package maintainer, biocon...

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:

61d9644 changed maintainer email to one without symbols as...

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

e8993ae changed gitignore to include documentation files, ...

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.

Neurergus commented 5 years ago

Hi, could I get an update on how the review process is going? Do I need to do something else? Thanks.

Liubuntu commented 5 years ago

Hi @Neurergus ,

Sorry for the delay. I'm reviewing it right now. Should be posting the official review shortly.

Thanks, Qian

Liubuntu commented 5 years ago

Hi @Neurergus ,

It was pleasant reading your package. It is really nicely organized, defined and explained in the vignette.

A basic but important question, have you directly compared your tool with the other existing R/Bioc packages that do simulation of RNA-seq data? such as Polyester. If yes, what's the difference in the strategies and any advantages?

I have some comments on several sections:

DESCRIPTION

Please remove LazyData = true. This rarely proves to be a good thing. In our experience it only slows down the loading of packages with large data.
BugReports: field: It is encouraged to include the relevant links to Github for reporting Issues.
Recommended to add URL field, to direct users to source code repositories, e.g., the GitHub repository.

man/

Generics.Rd. There are multiple of replicate arguments defined in the same documentation file. So if the argument represents same thing in different functions, you can just define them once. Otherwise, in the documentation contents following e.g., @param object, add something to indicate which function it was related. E.g., @param object in 'simSettings': Object of class .... Check other argument documentations.

Generate your real help page from "Generics.Rd", you will see many sections such as "Shared methods", and "Simulator methods", that don't have very clear indication of which generic functions each sentence of explanation refers to. Add some labels as I suggested above for clearer organization of the joint documentation.

The above review are not a complete review of the whole package. I have looked at the vignette, some R functions (classes, generic methods, help pages, etc.), generally they looks really nicely done. I'll have to look deeper to get a more complete idea, and will be posting more review points later.

Let me know for any questions.

Best, Qian

dvantwisk commented 5 years ago

Hi @Neurergus

Since @Liubuntu is out, she asked me to finish up the review. I've reviewed your code and it looks excellent. I really only have a few additional issues that need to be addressed. [REQUIRED] tags must be addressed while [CONSIDER] tags are describe changes that are advisable but not necessary. Please message us back once you finish making changes.

GENERAL

[ ] [CONSIDER] Your respository is about 30MB in size. We generally like to see repositories no larger than 5MB. Your .git directory is 27MB in size indicating you may have committed larger files, then deleted them. I recommend that you use BFG-repo-cleaner https://rtyley.github.io/bfg-repo-cleaner/ to reduce your repository size as much as possible. We will accept larger repositories if they cannot have their size further reduced.

R/AllClass.R

[ ] [REQUIRED] Please be more specfic with exported class names. The Biocondcutor suite includes a large ecosystem of packages and we'd like to see the more general names reserved for higher level functionality. In particular, I ask the class names like Simulation be changed to something like MOSimulation so that they are specific to your package. Given your generics are internal, you do not need to modify their names.

R/MOSim.R

[ ] [REQUIRED] L102: Error says list, should be vector.

bioc-issue-bot commented 5 years ago

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

099b36c changed name of classes. restored original tmax ra... 36a7569 removed generics page from man folder b819579 version bump

Neurergus commented 5 years ago

Hi Liubuntu & dvantwisk, thank you very much for taking the time to review the package and for your kind words..

Answering Liubuntu question: yes, we have compared our method with others. The main difference with Polyester is that MOSim is designed to simulate multiple omics but also -and more importantly- the regulatory programs between them. So the regulatory omics data (DNase-seq, ChIP-seq...) is correlated with gene expression data (RNA-seq). If specified, MOSIm could simulate only RNA-seq, however the output format between the two simulators is different: MOSim returns the count data matrix and Polyester the fasta files.

I have prevented roxygen from generating the generics man page, it was already internal and not visible in the index but it was not really useful to the final user anyway.

I have also renamed "Simulation" and "Simulator" classes to "MOSimulation" and "MOSimulator" and modified the DESCRIPTION.

The only thing I have not done yet is cleaning the repository as is marked as CONSIDER and I would fist like to check with more detail the linked tool.

Thanks again.

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.

Liubuntu commented 5 years ago

Thanks Dan @dvantwisk for the help!

Hi @Neurergus ,

to be consistent, would you want to rename the S4 class SimulatorRegion as MOSimulatorRegion?

For the warning:

S4 class codoc mismatches from documentation object 'MOSimulator-class':
Slots for class 'MOSimulator'
  Code: data depth depthAdjust depthRound idToGene increment max min
        minMaxDist minMaxFC minMaxQuantile name noiseFunction
        noiseParams pregenerated randData regulator regulatorEffect
        replicateParams roundDigits simData totalFeatures
  Docs: data depth depthAdjust depthRound idToGene increment max min
        name noise noiseFunction noiseParam pregenerated regulator
        regulatorEffect simData totalFeatures

Please check the setdiff() about these 2 sets. One example, the minMaxDist, randData, replicateParams, roundDigits was not documented, but the minMaxQuantile was documented twice, etc. The noise and noiseParam are inside the Rd file, but does not exist in the class slots...

Strongly recommended:

* Checking coding practice...
    * NOTE: Avoid sapply(); use vapply()

Let me know for any questions.

Best, Qian

Liubuntu commented 5 years ago

Hi @Neurergus ,

Are you working on any updates of this package? Please make necessary updates / commits / comments here so that we know the development is active. Thanks!

Best, Qian

Neurergus commented 5 years ago

Hi @Liubuntu ,

Sorry, due to holidays I do not have access to my workstation and I could not work properly so I decided to wait. I should return at the end of next week but yes, the development is still active.

Again, sorry for the inconvenience.

Cheers.

bioc-issue-bot commented 5 years ago

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

e62a406 renamed SimulatorRegion class to MOSimulatorRegion ae42b4a fixed some check warnings eb0b38d 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.

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/Neurergus.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 BiocManager::install("MOSim"). The package 'landing page' will be created at

https://bioconductor.org/packages/MOSim

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

MOSim #1164

DESCRIPTION

man/