search

Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

TimeScape Contribution #167

Closed maiasmith closed 7 years ago

maiasmith commented 8 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 8 years ago

Hi @maiasmith

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: timescape
Title: Patient Clonal Timescapes
Version: 0.99.0
Authors@R: person("Maia", "Smith", email = "maiaannesmith@gmail.com", role = c("aut", "cre"))
Description: Visualizes clonal evolution over time
Depends:
    R (>= 3.2.2)
Imports:
    htmlwidgets (>= 0.5),
    jsonlite (>= 0.9.19),
    stringr (>= 1.0.0)
biocViews: Visualization
License: GPL-3
LazyData: true
RoxygenNote: 5.0.1
Suggests: knitr, rmarkdown
VignetteBuilder: knitr
bioc-issue-bot commented 8 years ago

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

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

maiasmith commented 8 years ago

I've fixed the error (just a subscription to bioc-devel) - can I re-submit this package now?

mtmorgan commented 8 years ago

yes, if you have set up the web hook then bump the version to trigger a re-build.

bioc-issue-bot commented 8 years ago

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

2234210 updated version number

bioc-issue-bot commented 8 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 following build report for more details:

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

bioc-issue-bot commented 8 years ago

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

d3578f4 trigger rebuild

bioc-issue-bot commented 8 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 following build report for more details:

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

bioc-issue-bot commented 8 years ago

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

a3af1f9 fixed documentation for bioconductor

bioc-issue-bot commented 8 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 following build report for more details:

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

mtmorgan commented 7 years ago

23 December, 2016

I'm sorry to be negligent in reviewing these exciting packages.

I agree with Michael that more rigorous data modeling would be appropriate, but do no wish to hold up processing this package further. I offer the following suggestions:

DESCRIPTION

NAMESPACE

R

man

vignettes

misc

mtmorgan commented 7 years ago

To elaborate on the vignette comments. The visualization tools provided by the package are really great. To use the tools requires highly structured data. It is therefore essential to provide an unambigous path from the original data source(s) to the inputs used in your functions.

The vignette is a great place to do this -- provide details on how one obtains the inputs AML_tree_edges.csv, AML_clonal_prev.csv, AML_mutations.csv. Preferably, these steps use or highlight R processing and Bioconductor tools.

Well-supported Bioconductor concepts should emphasize Bioconductor tools ad data representations. Typical examples include working with called variants (parsing VCF files with VariantAnnotation) and genomic ranges (GRanges objects from the GenomicRanges package), and coordinated management of assays and sample phenotypic data (SummarizedExperiment from the SummarizedExperiment package).

bioc-issue-bot commented 7 years ago

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

129da6e updated for bioconductor bd241f2 Merge pull request #1 from mas29/master updated f...

bioc-issue-bot commented 7 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 following build report for more details:

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

bioc-issue-bot commented 7 years ago

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

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

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

46e7313 version bump

bioc-issue-bot commented 7 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 following build report for more details:

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

maiasmith commented 7 years ago

Hi Martin,

Thanks for your enthusiasm. I'm working through these, starting with TimeScape. I believe I have addressed all your points with regard to this package. Please let me know if this has passed the test!

Also, I've greatly updated the vignette, but am forgetting how to build it such that the docs folder appears with the vignette inside...?

Best,

Maia

On Sat, Dec 24, 2016 at 5:55 AM, Martin Morgan notifications@github.com wrote:

To elaborate on the vignette comments. The visualization tools provided by the package are really great. To use the tools requires highly structured data. It is therefore essential to provide an unambigous path from the original data source(s) to the inputs used in your functions.

The vignette is a great place to do this -- provide details on how one obtains the inputs AML_tree_edges.csv, AML_clonal_prev.csv, AML_mutations.csv. Preferably, these steps use or highlight R processing and Bioconductor tools.

Well-supported Bioconductor concepts should emphasize Bioconductor tools ad data representations. Typical examples include working with called variants (parsing VCF files with VariantAnnotation https://bioconductor.org/packages/VariantAnnotation) and genomic ranges (GRanges objects from the GenomicRanges https://bioconductor.org/packages/GenomicRanges package), and coordinated management of assays and sample phenotypic data ( SummarizedExperiment from the SummarizedExperiment https://bioconductor.org/packages/SummarizedExperiment package).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-269085322, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIqmLDGo_F36sh_4lD952NC7R2ptCLks5rLSQ9gaJpZM4KXcrX .

mtmorgan commented 7 years ago

Thanks Maia for your work. The usual protocol is to build then install

R CMD build timescape
R CMD INSTALL timescape_0.99.5.tar.gz

The build command creates the vignette. Your git repository does not need to (should not) have an inst/docs folder. You can also use devtools and, when cwd() is somewhere inside your package source directory, install(build_vignettes=TRUE).

bioc-issue-bot commented 7 years ago

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

da2939c updated vignette

bioc-issue-bot commented 7 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 following build report for more details:

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

maiasmith commented 7 years ago

Thanks Martin.

I believe I have made corrections for the three packages: TimeScape, MapScape and CellScape. Please keep me posted on the status of these.

Best, Maia

On Sat, Dec 24, 2016 at 4:26 PM, Martin Morgan notifications@github.com wrote:

Thanks Maia for your work. The usual protocol is to build then install

R CMD build timescape R CMD INSTALL timescape_0.99.5.tar.gz

The build command creates the vignette. Your git repository does not need to (should not) have an inst/docs folder. You can also use devtools https://cran.r-project.org/package=devtools and, when cwd() is somewhere inside your package source directory, install(build_vignettes=TRUE).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-269105403, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIqnOYVs8jra_6HJxJl7YIdXVqJd3Fks5rLbhBgaJpZM4KXcrX .

mtmorgan commented 7 years ago

Thank you for the updates. I have the remaining concerns that I hope you can easily address.

vignette

Please continue to provide more information on the upstream work flow.

Other

Please review files in the repository, especially top level, to include only those that are required for the package. E.g., 3q seems like a stray. Are the three README files really necessary?

man (Optional)

The man pages are formatted in a fairly ad-hoc way, so that their presentation is not really optimal. For instance white space is compressed so this relatively legible specification

#' @param clonal_prev (Data Frame) Clonal prevalence. Note: timepoints will be alphanumerically sorted in the view.
#'   Format: columns are (1) character() "timepoint" - time point
#'                       (2) character() "clone_id" - clone id
#'                       (3) numeric() "clonal_prev" - clonal prevalence.

is displayed as

clonal_prev: (Data Frame) Clonal prevalence. Note: timepoints will be
          alphanumerically sorted in the view. Format: columns are (1)
          character() "timepoint" - time point (2) character()
          "clone_id" - clone id (3) numeric() "clonal_prev" - clonal
          prevalence.

Improvements would

For instance

#' @param clonal_prev \code{data.frame} Clonal prevalence. Required
#'     columns are:
#'     \describe{
#'
#'       \item{timepoint:}{\code{character()} time point. Time
#'          points will be alphanumerically sorted in the view.}
#' 
#'       \item{clone_id:}{\code{character()} clone id.}
#' 
#'       \item{clonal_prev:}{\code{numeric()} clonal prevalence.}
#'
#'     }

Likewise in the Details section.

#' Interactive components:
#'   \enumerate{
#' 
#'     \item Mouseover any clone to view its (i) clone ID and (ii) clonal
#'     prevalence at each time point.
#' 
#'     \item Click the view switch button to switch from the traditional
#'     timescape view to the clonal trajectory view, where each clone
#'     changes prevalence on its own track.
#' 
#'     \item Click the download buttons to download a PNG or SVG of the
#'     view.
#' 
#'   }

Please also consider line width in the man page (suggested maximum: 80 columns), especially the examples. For instance, in a standard 80 column terminal (yes, old-school, but not an uncommon way of using R) the examples currently wrap in an ugly way

     # EXAMPLE 1 - Acute myeloid leukemia patient, Ding et al., 2012

     # genotype tree edges
     tree_edges <- read.csv(system.file("extdata", "AML_tree_edges.csv", package
 = "timescape"))

They are more readable with

#' @examples
#'
#' # EXAMPLE 1 - Acute myeloid leukemia patient, Ding et al., 2012
#'
#' # genotype tree edges
#' tree_edges <- read.csv(system.file("extdata", "AML_tree_edges.csv",
#'     package = "timescape"))
mtmorgan commented 7 years ago

Also, be sure to remove 'hidden' files like .DS_store in the root and vignettes directory.

maiasmith commented 7 years ago

Hi Martin,

Could you please provide clarification as to what kind of description is necessary for the data preparation? A full description of how to use PyClone and CITUP is quite intensive, the subject of numerous supplemental pages in both papers.

I'm CC'ing Andrew McPherson, a developer of CITUP, who will help me develop an appropriate description of the methods, but is seeking further clarification on the requirements here.

Best, Maia

On Mon, Jan 9, 2017 at 3:01 AM, Martin Morgan notifications@github.com wrote:

Thank you for the updates. I have the remaining concerns that I hope you can easily address. vignette

Please continue to provide more information on the upstream work flow.

  • Link to software source code for PyClone and to CITUP.
  • Provide explicit instructions for use of this software, e.g., the format of the original (raw) data and the exact commands / configuration variables used to create the data analyzed in this package.
  • The exact relationship between the output files of these programs and the input files used by your software.

Other

Please review files in the repository, especially top level, to include only those that are required for the pacakge. E.g., 3q and the *png files seem like strays. man (Optional)

The man pages are formatted in a fairly ad-hoc way, so that their presentation is not really optimal. For instance white space is compressed so this relatively legible specification

' @param clonal_prev (Data Frame) Clonal prevalence. Note: timepoints will be alphanumerically sorted in the view.

' Format: columns are (1) character() "timepoint" - time point

' (2) character() "clone_id" - clone id

' (3) numeric() "clonal_prev" - clonal prevalence.

is displayed as

clonal_prev: (Data Frame) Clonal prevalence. Note: timepoints will be alphanumerically sorted in the view. Format: columns are (1) character() "timepoint" - time point (2) character() "clone_id" - clone id (3) numeric() "clonal_prev" - clonal prevalence.

Improvements would

  • use \code{data.frame} (standard R terminology, rather than Data Frame), \code{character()} etc. to mark up R code.
  • use \describe{} to create a descriptive list of parameters.
  • elaborate on details of each column.

For instance

' @param clonal_prev \code{data.frame} Clonal prevalence. Required

' columns are:

' \describe{

'

' \item{timepoint:}{\code{character()} time point. Time

' points will be alphanumerically sorted in the view.}

'

' \item{clone_id:}{\code{character()} clone id.}

'

' \item{clonal_prev:}{\code{numeric()} clonal prevalence.}

'

' }

Likewise in the Details section.

' Interactive components:

' \enumerate{

'

' \item Mouseover any clone to view its (i) clone ID and (ii) clonal

' prevalence at each time point.

'

' \item Click the view switch button to switch from the traditional

' timescape view to the clonal trajectory view, where each clone

' changes prevalence on its own track.

'

' \item Click the download buttons to download a PNG or SVG of the

' view.

'

' }

Please also consider line width in the man page (suggested maximum: 80 columns), especially the examples. For instance, in a standard 80 column terminal (yes, old-school, but not an uncommon way of using R) the examples currently wrap in an ugly way

 # EXAMPLE 1 - Acute myeloid leukemia patient, Ding et al., 2012

 # genotype tree edges
 tree_edges <- read.csv(system.file("extdata", "AML_tree_edges.csv", package

= "timescape"))

They are more readable with

' @examples

'

' # EXAMPLE 1 - Acute myeloid leukemia patient, Ding et al., 2012

'

' # genotype tree edges

' tree_edges <- read.csv(system.file("extdata", "AML_tree_edges.csv",

' package = "timescape"))

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-271258004, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIqgehAShwp9MYMxN0FAfgpQb6dI4Aks5rQhOagaJpZM4KXcrX .

mtmorgan commented 7 years ago

Sorry for my slow response. I'm not looknig for anything too elaborate, a 'paragraph' that tells the user what the starting data were, what commands and parameter values were used, and what output file and additional processing were needed to make it appropriate for input into your package.

Please don't forget the version bump when you're ready.

bioc-issue-bot commented 7 years ago

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

6eeebd2 version bump

bioc-issue-bot commented 7 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 following build report for more details:

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

mtmorgan commented 7 years ago

@maiasmith I wasn't exactly sure where we stand on this. I do not see revisions to the vignette since you asked for clarification, but I am willing to accept the packages as they now stand. If of course you have revisions that you are about to submit then I will wait for them...

maiasmith commented 7 years ago

Hi Martin,

I updated the "Obtaining the data" section for Time/Map/CellScape packages, as you requested. Can you see it? In TimeScape, it should be a paragraph that begins as so: "E-scape takes as input a clonal phylogeny and clonal prevalences per clone per sample."

Please let me know if you see otherwise!

Maia

On Thu, Feb 9, 2017 at 2:33 PM, Martin Morgan notifications@github.com wrote:

@maiasmith https://github.com/maiasmith I wasn't exactly sure where we stand on this. I do not see revisions to the vignette since you asked for clarification, but I am willing to accept the packages as they now stand. If of course you have revisions that you are about to submit then I will wait for them...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-278796646, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIquKYFvPV4RWCxlkSUveuQNzmgHQgks5ra5RUgaJpZM4KXcrX .

mtmorgan commented 7 years ago

Sorry I some how misinterpreted those commits. I'm accepting these packages; they will be added to the Bioconductor svn repository and nightly builds in the next day or so, with more information forwarded when that is done.

mtmorgan commented 7 years ago

And thanks both for your contribution and patience!

maiasmith commented 7 years ago

Hi Martin,

Looks like the packages are out now - great!

However, when I try to install them, I am receiving an error - is this something I'm doing wrong, or something wrong with the packages?

source("https://bioconductor.org/biocLite.R") biocLite("timescape") BioC_mirror: https://bioconductor.org Using Bioconductor 3.2 (BiocInstaller 1.20.3), R 3.3.2 (2016-10-31). Installing package(s) ‘timescape’ Warning message: package ‘timescape’ is not available (for R version 3.3.2)

Best, Maia

On Thu, Feb 9, 2017 at 3:16 PM, Martin Morgan notifications@github.com wrote:

And thanks both for your contribution and patience!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-278806454, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIqp5DXsa4zuCbuijhfL2nePsG9Mdiks5ra55FgaJpZM4KXcrX .

maiasmith commented 7 years ago

I've now tried with Bioconductor 3.4 and it still doesn't work:

source("http://bioconductor.org/biocLite.R") Bioconductor version 3.4 (BiocInstaller 1.24.0), ?biocLite for help biocLite("mapscape") BioC_mirror: https://bioconductor.org Using Bioconductor 3.4 (BiocInstaller 1.24.0), R 3.3.2 (2016-10-31). Installing package(s) ‘mapscape’ Warning message: package ‘mapscape’ is not available (for R version 3.3.2)

On Sun, Feb 19, 2017 at 12:17 PM, Maia Smith mas29@sfu.ca wrote:

Hi Martin,

Looks like the packages are out now - great!

However, when I try to install them, I am receiving an error - is this something I'm doing wrong, or something wrong with the packages?

source("https://bioconductor.org/biocLite.R") biocLite("timescape") BioC_mirror: https://bioconductor.org Using Bioconductor 3.2 (BiocInstaller 1.20.3), R 3.3.2 (2016-10-31). Installing package(s) ‘timescape’ Warning message: package ‘timescape’ is not available (for R version 3.3.2)

Best, Maia

On Thu, Feb 9, 2017 at 3:16 PM, Martin Morgan notifications@github.com wrote:

And thanks both for your contribution and patience!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-278806454, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIqp5DXsa4zuCbuijhfL2nePsG9Mdiks5ra55FgaJpZM4KXcrX .

mtmorgan commented 7 years ago

Packages are added to the 'bioc-devel' branch, which is currently 'Bioc 3.5' and 'R-devel'. Bioc 3.5 will become the next release some time in April, at about the time 'R-devel' is released as R-3.4.0. See here.

maiasmith commented 7 years ago

Hi Martin,

Thanks for your previous response, I managed to get the bioc-devel branch up and running thanks to your help.

One further question - I'm not sure I follow how to update my software. I made a small change to the vignette in my cellscape repo on github (maiasmith), however this will not translate to the bioc devel version of my package because I'm missing which steps? The instructions I received in my email refer to the svn repo, but I require a username and password for this, which I'm not sure I have? Also, I would prefer to perform changes through git, if possible, as I've never used svn before. Can I simply push changes to the https://github.com/Bioconductor-mirror/cellscape repo?

Best,

Maia

On Mon, Feb 20, 2017 at 2:09 PM, Martin Morgan notifications@github.com wrote:

Packages are added to the 'bioc-devel' branch, which is currently 'Bioc 3.5' and 'R-devel'. Bioc 3.5 will become the next release some time in April, at about the time 'R-devel' is released as R-3.4.0. See here http://bioconductor.org/developers/how-to/useDevel/.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/167#issuecomment-281193657, or mute the thread https://github.com/notifications/unsubscribe-auth/ARaIqkH16RGzorznwBqumNb1fkLPvkNkks5reg8vgaJpZM4KXcrX .

mtmorgan commented 7 years ago

No unfortunately you need to arrange for your changes to be in the bioconductor svn repository, perhaps following the instructions at http://bioconductor.org/developers/how-to/git-mirrors/ . The basics of svn are at http://bioconductor.org/developers/how-to/source-control/ . The svn username and password were included in the email you reference above; same username / password for each package. contact me directly martin.morgan at roswellpark.org if you need the user name / password again. Better to ask on the bioc-devel mailing list, so others benefit / can help.