YAPSA

[huebschm]

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

• Repository: https://github.com/huebschm/YAPSA

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

• I understand that by submitting my package to Bioconductor,
  the package source and all review commentary are visible to the
  general public.
• I have read the Bioconductor Package Submission
  instructions. My package is consistent with the Bioconductor
  Package Guidelines.
• My package addresses statistical or bioinformatic issues related
  to the analysis and comprehension of high throughput genomic data.
• 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:

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

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

[bioc-issue-bot]

Hi @huebschm

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: YAPSA
Type: Package
Title: Yet Another Package for Signature Analysis
Version: 0.99.0
Date: 2015-08-04
Author: Daniel Huebschmann, Zuguang Gu, Matthias Schlesner
Maintainer: Daniel Huebschmann <huebschmann.daniel@googlemail.com>
Imports:
    limSolve,
    SomaticSignatures,
    VariantAnnotation,
    GenomeInfoDb,
    reshape2,
    gridExtra,
    corrplot,
    dendextend,
    GetoptLong,
    circlize (>= 0.3.8),
    gtrellis,
    PMCMR,
    ComplexHeatmap (>= 1.11.5),
    KEGGREST,
    grDevices
Depends:
    GenomicRanges,
    ggplot2,
    grid
Description: This package provides functions and routines useful in
    the analysis of somatic signatures (cf. L. Alexandrov et al., Nature
    2013). In particular, functions to perform a signature analysis with
    known signatures (LCD = linear combination decomposition) and a
    signature analysis on stratified mutational catalogue (SMC = stratify
    mutational catalogue) are provided.
License: GPL-3
Suggests:
    BSgenome.Hsapiens.UCSC.hg19,
    testthat,
    BiocStyle,
    knitr,
    rmarkdown
VignetteBuilder: rmarkdown
LazyLoad: yes
biocViews: Sequencing, DNASeq, SomaticMutation, Visualization,
    Clustering, GenomicVariation, StatisticalMethod, BiologicalQuestion
RoxygenNote: 5.0.1

[bioc-issue-bot]

Your package has been approved for building. Your package is
now submitted to our queue.

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]

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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160725155701.html

[bioc-issue-bot]

We only start builds when the Version field in the DESCRIPTION
file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to
do anything. If you did want to start a build, increment
the Version: field and try again.

[bioc-issue-bot]

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

d3d422e Now with version bump

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160726143645.html

[bioc-issue-bot]

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

8d750bc second revision Bioconductor: remove YAPSA.html

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160727093349.html

[bioc-issue-bot]

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

5fc893d subscribed to Bioc-devel mailing list

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160727102405.html

[bioc-issue-bot]

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

3123ea9 changed R version to 3.3.0 because of limSolve
008f2e0 R version 3.3.0 and version bump

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160727111256.html

[huebschm]

@hpages
Dear Bioconductor team,
after submission of my package, building and testing works well on zin1 (Linux) and morelia (Mac OS), but building fails on moscato (Windows):

moscato1 CHECK output:

• installing source package 'YAPSA' ...
  ** R
  ** data
  ** inst
  ** preparing package for lazy loading
  Warning: replacing previous import 'ggplot2::Position' by 'BiocGenerics::Position' when loading 'ggbio'
  Error in library.dynam(lib, package, package.lib) :
  DLL 'limSolve' not found: maybe not installed for this architecture?
  ERROR: lazy loading failed for package 'YAPSA'
• removing 'D:/packagebuilder/workers/jobs/YAPSA_20160727145534/YAPSA.buildbin-libdir/YAPSA'
  Warning: running command 'D:/packagebuilder/R/bin/i386/Rcmd.exe INSTALL --library=YAPSA.buildbin-libdir YAPSA_0.99.4.tar.gz --no-multiarch' had status 1

Looking at the output of the build of other packages, e.g.
http://bioconductor.org/checkResults/release/bioc-LATEST/moscato2-R-instpkgs.html
reveals that the package limSolve, which my package depends on, is installed under R-3.3.0:
limSolve 1.5.5.1 3.3.0

Is there anything that can be done to overcome this?
Thank you very much in advance for your help, best Daniel.

[vobencha]

Hi Daniel,

YASPA has been transferred to me and I'll be doing the review. I wanted to make sure you saw Martin's response to your questions here

https://stat.ethz.ch/pipermail/bioc-devel/2016-July/009570.html

Valerie

[huebschm]

Dear Valeria,
thank you very much for reviewing YAPSA. Indeed I saw Martin's response (thank you very much Martin). I tried out the following:

• install.packages("limSolve") in R-3.3.1 on Windows 10

This worked out, so comparing to the report under
https://www.r-project.org/nosvn/R.check/r-release-windows-ix86+x86_64/limSolve-00check.html
it seems like it is only an example of limSolve which throws the error.
Of course I can still try to contact the authors of limSolve.
Best Daniel.

[vobencha]

I don't understand what you mean by 'this worked out'.

You have 2 choices:

1. use a different dependency (something other than limsolve)
2. contact the author of limSolve to fix the error
   Until one of these two is done YASPA will not pass check on windows and cannot be approved.

Valerie

[huebschm]

Dear Valerie,
by 'this worked out' I mean that install.packages("limSolve") in R-3.3.1 on Windows 10 doesn't throw an error, and the package limSolve can be loaded and used with e.g. library(limSolve).
But of course, I will try to solve the build problem and am contacting the maintainer of limSolve.
Best, Daniel.

[vobencha]

Hi,
Just checking in. Have you found a solution to the limSolve problem ... are you ready for me to do a full review?
Valerie

[huebschm]

Hi, the maintainer of limSolve didn't reply. Therefore we were looking for an alternative. It seems like limSolve uses the package lsei, and for the functions we need lsei might be sufficient. What I don't know yet is if lsei builds on windows, but we will try that out.
Best Daniel

[bioc-issue-bot]

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

36f97f9 replace limSolve dependency by lsei

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160812161544.html

[bioc-issue-bot]

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

9150e96 changed expect_less_than to expect_lt
dfb162a further changes in expect_less_than to expect_lt

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160812181144.html

[bioc-issue-bot]

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

0ccccff changed test in LCD_SMC from matrix to data frame

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160812212020.html

[bioc-issue-bot]

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

9c6253b reformatted test_LCD

[bioc-issue-bot]

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160813060738.html

[bioc-issue-bot]

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

efc6651 further adaptions on test LCD_SMC

[bioc-issue-bot]

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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160813074353.html

[huebschm]

Dear Valerie,
I replaced the dependency limSolve by lsei and now there is no more build error on windows.
Is this a state in which you could start your review?
Thank you very much, best regards Daniel.

[vobencha]

Yes, that's good, thanks. I'm looking at it now and should have comments for you by tomorrow.
Valerie

[vobencha]

Great job with the vignette and code organization. Comments and suggestions below.

(1) number of exported functions
My biggest concern is that you're exporting 79 objects
~/YAPSA >grep "export" NAMESPACE | wc -l
79

and have over 100 man pages:
~/proj/git/software/YAPSA/man >ls -l | wc -l
108

In general, the number of exported objects should be >= the number of man pages. Everything that is exported needs a man page but if a function is not exported it does not need a man page.

Is it essential the user have access to all these functions? If some (many?) are for internal use only it would be a much cleaner design to not export (and not document) these functions. Please explain.

(2) support VRAnges

Several functions take as input a data.frame of chromosome name, position, alt, ref etc. which is a natural representation for VRanges. As you probably know, VRanges is a commonly used class for variants and supporting this would make it easy for users with data already in VRanges to use your functions.

For functions that operate on this type of data.frame please add support for VRanges. For example, in trellis_rainfall_plot.Rd the 'in_rainfall_dat' argument can be either a data.frame or VRanges.

I'm sure I've missed a few but it looks like these functions support this type of data.frame and should also support VRanges:
trellis_rainfall_plot.Rd
translate_to_hg19.Rd
translate_to_1kG.Rd
annotate_intermut_dist_cohort.Rd
annotate_intermut_dist_PID.Rd
attribute_nucleotide_exchanges.Rd

(3) Please format your code to max 80 characters wide.

(4) non-evaluated man pages

Please explain why these are wrapped in \dontrun:

~/proj/git/software/YAPSA/man >grep dont *
build_gene_list_for_pathway.Rd: \dontrun{
create_mutation_catalogue_from_df.Rd:\dontrun{
create_mutation_catalogue_from_VR.Rd:\dontrun{
cut_breaks_as_intervals.Rd: \dontrun{
run_SMC.Rd:\dontrun{
stratify_and_create_mutational_catalogue.Rd:\dontrun{
stratify_vcf_like_df.Rd:\dontrun{
trellis_rainfall_plot.Rd:\dontrun{

(5) vignette

• Please download and save this figure as a file instead of trying to retrieve it from the web site:
  https://upload.wikimedia.org/wikipedia/commons/f/f9/NMF.png
• Add a sentence or two in this introduction paragraph that states what YAPSA offers beyond SomaticSignatures:
  "Another package to perform analyses of mutational signatures is available (Gehring et al. 2015) ..."

Let me know if you have questions.

Valerie

[bioc-issue-bot]

We only start builds when the Version field in the DESCRIPTION
file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to
do anything. If you did want to start a build, increment
the Version: field and try again.

[huebschm]

Dear Valerie,
thank you very much for your review and your comments. I am almost done with the changes and will post an explanation of the changes once ready.
Best Daniel.

[bioc-issue-bot]

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

197fe57 last changes for code review

[huebschm]

Dear Valerie,
I submitted a new version which complies with all of your comments and suggestions:

(1) number of exported functions:

• The number of exported functions was reduced:
  ~/software/huebschm/R/yapsa> grep export NAMESPACE | wc -l
  69
  Technically, it could be reduced further (there are e.g. a few wrapper functions and the functions they call are still exported), but as I use YAPSA in the data evaluation for a few projects, I get the feeling that usability decreases if I reduce the namespace further.
• The number of man pages was also reduced:
  ~/software/huebschm/R/yapsa> ls -al man/ | wc -l
  68
  Initially there were many separate man pages for the data, but these (as well as some man pages for functions) are now grouped into more comprehensive man pages describing several objects at once.

(2) support VRAnges

• trellis_rainfall_plot.Rd: now supports data frames and VRanges as input
• translate_to_hg19.Rd: now supports data frames and VRanges as input
• translate_to_1kG.Rd: now supports data frames and VRanges as input
• annotate_intermut_dist_cohort.Rd: now supports data frames and VRanges as input
• annotate_intermut_dist_PID.Rd: now supports data frames and VRanges as input
• attribute_nucleotide_exchanges.Rd: now supports VRanges, VRangesList, data frames or lists of data frames as input
• make_subgroups_df.Rd: now supports data frames and VRanges as input

(3) Please format your code to max 80 characters wide.
I reformatted all lines which are accessible to me. There are a few exceptions which still are longer than 80 characters:

• some lines in the documentation paragraphs include urls which are longer and cannot be broken
• the lines defining chunk names and chunk options in Rmd in the vignette
• the Rd files (automatically generated by roxygenize(), therefore I haven't edited these manually)

(4) non-evaluated man pages

• build_gene_list_for_pathway.Rd: \dontrun{: only example left with dontrun. This is a wrapper function for keggFind() and keggLink() from the KEGGREST package. In the example a query is sent to KEGG and the time needed for the evaluation of this example depends on connectivity of the user. Therefore I would suggest to leave this in dontrun{}.
• create_mutation_catalogue_from_df.Rd:\dontrun{: dontrun removed
• create_mutation_catalogue_from_VR.Rd:\dontrun{: dontrun removed
• cut_breaks_as_intervals.Rd: \dontrun{: dontrun removed
• run_SMC.Rd:\dontrun{: dontrun removed
• stratify_and_create_mutational_catalogue.Rd:\dontrun{: function not exported any more
• stratify_vcf_like_df.Rd:\dontrun{: function not exported any more
• trellis_rainfall_plot.Rd:\dontrun{: dontrun removed

(5) vignette

• Please download and save this figure as a file instead of trying to retrieve it from the web site: https://upload.wikimedia.org/wikipedia/commons/f/f9/NMF.png: done
• Add a sentence or two in this introduction paragraph that states what YAPSA offers beyond SomaticSignatures: "Another package to perform analyses of mutational signatures is available (Gehring et al. 2015) ...": done

Thank you very much for your comments and suggestions, best Daniel.

[huebschm]

Dear Valerie,
I have the impression that since my last commit, no build has been finished, even though the message

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

197fe57 last changes for code review

was sent out. Could this be related to the fact that by increasing the version number I reached 0.99.10 by now, i.e. two digits after the last separator?

Also by mistake I had closed the issue and reopened it right again.

Thank you for your help, best Daniel.

[vobencha]

Thanks for making the changes and documenting what you did. I'm not sure why the SPB didn't complete - I'll check with Lori. The 'z' portion of the 'x.y.z' version can go as high as you want; there is no restriction on that. More than likely it was related to the closing of the issue. Once we get a clean build/check I'll approve the package.

Valerie

[bioc-issue-bot]

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 following build report for more details:

http://bioconductor.org/spb_reports/YAPSA_buildreport_20160822104316.html

[huebschm]

Dear Valerie,
the package has now been built. Thank you for triggering the process. Is the current state sufficient for approval?
Thank you very much again, best regards Daniel.

[vobencha]

Yes, looks good. You'll get an email from Martin within the next week with svn credentials and next steps. Thanks again for putting in the extra work to improve the package.
Valerie