Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

CellTrails #743

Closed dcellwanger closed 6 years ago

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

Hi @dcellwanger

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: CellTrails
Type: Package
Title: Reconstruction, visualization and analysis of branching trajectories
Version: 0.99.0
Author: Daniel Christian Ellwanger [aut,cre]
Authors@R: person(given="Daniel", family="Ellwanger", 
  role=c("aut", "cre"), email="dc.ellwanger.dev@gmail.com")
Maintainer: Daniel Christian Ellwanger <dc.ellwanger.dev@gmail.com>
Description: CellTrails is an unsupervised algorithm for the de novo 
  chronological ordering, visualization and analysis of single-cell expression 
  data. CellTrails makes use of a geometrically motivated concept of 
  lower-dimensional manifold learning, which exhibits a multitude of virtues that 
  counteract intrinsic noise of single cell data caused by drop-outs, technical 
  variance, and redundancy of predictive variables. CellTrails enables the 
  reconstruction of branching trajectories and provides an intuitive graphical 
  representation of expression patterns along all branches simultaneously. It allows 
  the user to define and infer the expression dynamics of individual and multiple 
  pathways towards distinct phenotypes.
License: Artistic-2.0 | file LICENSE
Encoding: UTF-8
LazyData: true
VignetteBuilder: knitr
Depends: R (>= 3.4), Biobase
Imports:
  BiocStyle,
  cba,
  dendextend,
  depth,
  dtw,
  EnvStats,
  ggplot2,
  ggrepel,
  grDevices,
  graphics,
  igraph,
  maptree,
  methods,
  mgcv,
  reshape2,
  Rtsne,
  stats,
  splines,
  viridis,
  utils
Suggests: 
  destiny, 
  scran, 
  knitr,
  rmarkdown
RoxygenNote: 6.0.1
biocViews: 
  Clustering,
  DataRepresentation,
  DifferentialExpression,
  DimensionReduction,
  GeneExpression,
  Sequencing,
  SingleCell,
  Software,
  TimeCourse
Collate: 
  'AllClasses.R'
  'AllGenerics.R'
  'accessor-methods.R'
  'cluster-methods.R'
  'constructor-methods.R'
  'data-sets.R'
  'diffexp-methods.R'
  'dimred-methods.R'
  'export-methods.R'
  'filter-methods.R'
  'fitting-methods.R'
  'import-methods.R'
  'imputation-methods.R'
  'layout-methods.R'
  'plot-methods.R'
  'show-methods.R'
  'trajectory-methods.R'
  'utility-methods.R'
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

@lshep will review this package, but it seems like it does not use the mature SingleCellExperiment representation or any of the related single cell and GenomicRanges packages in Bioconductor. Your package would be much more valuable to the community if it interoperated via this mature infrastructure.

I'm also concerned about the broad and seemingly redundant graphical-related dependencies, including as implied in the vignette on a third party project; are these really necessary for the core features of your project? Many dependencies make packages very fragile.

Your vignette is currently essentially one large code chunk with words at the beginning and end, but would be much better if the text, code, and output were integrated in a way that explains the operations being performed and the output produced.

Others in the single cell community (e.g., @LTLA @drisso ) might have additional comments.

LTLA commented 6 years ago

At least a few of your custom slots can be stored as int_metadata or int_elementMetadata or int_rowData, depending on whether it makes any sense for those slots to respond to subsetting. We are currently revising the developer instructions in SingleCellExperiment for defining new package-specific methods that use the internal data slots; this may be a good opportunity to get involved.

If the graph needs to be subsetted as well (not that I think it makes scientific sense to do so), it may also be worth considering the work that @dvantwisk has done for LoomExperiment.

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.

drisso commented 6 years ago

I agree with @LTLA that most if not all the custom slots can be fit naturally into a SingleCellExperiment object. If there is really a need for something special, you should at least extend SingleCellExperiment (or we may consider adding it to the SingleCellExperiment class, if general enough).

bioc-issue-bot commented 6 years ago

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

07dba51 Update 0.99.1

dcellwanger commented 6 years ago

Thank you, Martin Morgan (@mtmorgan), Aaron Lun (@LTLA) and Davide Risso (@drisso), for your initial comments and Lori Ann Shepherd (@lshep) for taking your time to review our package.

Our package accompanies a manuscript that is currently in its final review stage (it has been "accepted in principle" at Cell Reports). As soon as it gets formally accepted, we are happy to provide the article to Lori Ann, as it will help to understand the novelties of this package.

Regarding the issue on CellTrails’ dependency on a third-party project: CellTrails reconstructs a branching (developmental) trajectory from gene expression data and stores the trajectory information in a graph structure. Since we did not find a proper implementation of a planar graph embedding within R, we need to make use of a graph layout algorithm provided by third-party software. We implemented a function to export the reconstructed trajectory graph information into the generally accepted graphml graph file format, which can be interpreted by most graph analysis software packages. Please note that CellTrails requires this step only to properly draw the graph. Subsequent analyses and visualizations are computed in R; an import function is provided, respectively. Some of these additional useful but non-essential export/import functionalities were requested by early adopters of our package and these users unanimously agreed that they simplified the analysis of the trajectory graph using this third-party software.

Regarding the issue on the vignette. We would be very pleased if we could provide the detailed tutorial (see our CellTrails project pages site: https://dcellwanger.github.io/CellTrails/), as referenced in the vignette, along with the package. However, the compiled detailed tutorial is about 6MB and therefore triggered a file size warning on our local BiocCheck. Since we intend to extend the functionalities of the package in the future, which eventually will result in an increase of the vignette’s file size, we decided to provide a general overview of the workflow in the package vignette and reference to the project page for the detailed tutorial (similar to the structure of the monocle package [http://10.18129/B9.bioc.monocle]).

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.

dcellwanger commented 6 years ago

Regarding the SingleCellExperiment class. We truly appreciate Aaron’s and Davide’s valiant effort to implement a unified class for the analysis of single-cell expression data. When we started implementing the package, we decided to extend the ExpressionSet class, as its slots were sufficient to represent single-cell and population-based RT-qPCR and log-normalized RNA-Seq counts, and it nicely stored and printed the experimental meta-information. CellTrails is intended to be used downstream of QC and normalization and therefore, raw counts, spike-ins and size factor information are neglected. On the additional slots of a CellTrailsSet:

We also provide an implementation of show, which gives a digest of the information contained in a CellTrailsSet. Every slot shown can be accessed with a getter method. This is particularly helpful to keep track of each analysis step.

To keep the source code well-arranged, we would need to extend the SingleCellExperiment class, similarly to our current ExpressionSet extension. Additionally, we would need to implement the ExpressionSet getters (e.g., featureNames, exprs, usf.), since our package makes use of these functions. Therefore, we would be very grateful if we could perform this step in a later release of CellTrails. We extended the current package implementation by a function allowing the conversion of a SingleCellExperiment into a CellTrailsSet and vice versa (similar to scran::convertTo).

bioc-issue-bot commented 6 years ago

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

3cc095a Update 0.99.2

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

dcellwanger commented 6 years ago

Hi @lshep,

There is one WARNING left triggered by the rgl package (unable to open X11 display): 'rgl_init' failed, running with rgl.useNULL = TRUE. Please, let me know whether this issue can and needs to be resolved.

Thank you for your time. Daniel

mtmorgan commented 6 years ago

Extending ExpressionSet was an (understandable) design mistake; a modern approach would extends SummarizedExperiment, which is what SingleCellExperiment does. It doesn't make sense from a development perspective to release an inappropriate class intending to update it in the future -- the inappropriate class will be saved by various users, and they will be forced to update these and become familiar with your new representation. And of course speaking for myself it is very easy to intend to do something but not actually ever do it. It would be a mistake to implement ExpressionSet accessors (e.g., featureNames(), featureData()) instead of updating your code to use SummarizedExperiment / SingleCellExperiment accessors (e.g., rownames(), rowData()).

It might be surprisingly easy to update your class definition and implementation, the underlying conceptual building blocks are the same.

mtmorgan commented 6 years ago

With respect to your vignette, if the full vignettte processes in a timely manner then I would not worry about the small excess in the final size of your package. On the other hand if the vignette requires excessive computation then I will return to my original comment, which is that you provide a code chunk without context, and the context must be provided.

Your package does not have formal tests, so it is important that the code be evaluated somehow to avoid 'bit rot'. In lieu of tests, evaluated examples and vignettes become a key component of this step in software quality assurance.

I understand that users request features, and that it is very tempting to provide them. The down side of this is that your package becomes very fragile (because it depends on an increasingly large and rapidly changing code base outside your control) and difficult to maintain (because of the increasingly complex logical constraints your earlier design decisions impose on subsequent development). This is the voice of experience speaking, urging you to carefully consider each feature you wish to expose.

LTLA commented 6 years ago

As a voice of somewhat lesser experience, I'll second Martin's comment about tests, which allow me to sleep properly at night without worrying that about critical bugs in my 10+ packages.

The comment about features is also relevant; I've had a number of instances where I coded myself into a corner by allowing too much generality in the interface. This is especially tempting for plotting functions. My opinion is that high-level plotting functions are intended for quick-and-dirty visualisation throughout the course of an analysis; if users want full customisability with publication-quality figures, they should realistically assemble the plotting commands themselves.

As for SingleCellExperiment; one of the obvious advantages over ExpressionSet is that it can properly handle sparse and file-backed matrices (increasingly important for modern single-cell experiments with thousands of cells). As you've noticed, it also supports strong interoperability between packages, which really is important when your package only does one analysis step.

I don't think any of your slots are incompatible with a SingleCellExperiment. Indeed, if you just wanted to recapitulate your existing implementation, you could just store your new slots in the internal metadata (e.g, using a list named CellTrails), and allow the accessors to retrieve them by any arbitrary name. This is possible as your subset method doesn't acknowledge the new slots anyway (and in fact, returns the expression matrix rather than the S4 object, which is rather confusing to users).

drisso commented 6 years ago

Just to add another voice echoing @LTLA and @mtmorgan comments, I am really convinced that in the long term your package will benefit a lot from using (or inheriting from) SingleCellExperiment (or at least SummarizedExperiment). As Martin said, it is very little effort up-front to make this change now and it will save you a lot of headaches going forward.

I totally understand your point of view of having your own object, but I would strongly suggest that at least it inherits from SingleCellExperiment (or SummarizedExperiment) rather than ExpressionSet. If your object inherits from SingleCellExperiment, for instance, an user could simply use other packages for QC and normalization (which you said is not the focus of your package) and seamlessly integrate your package into their workflows without the need of any coercion methods.

One more thing about extending SingleCellExperiment vs just using it. As @LTLA said there is really nothing in your object that could not be fit into a SingleCellExperiment, but I acknowledge that with the current documentation of SingleCellExperiment this is not obvious. We are willing to work with you in making that more clear and using your package as a use case to write a "developer guide" on how to use SingleCellExperiment in one's package. For instance by defining methods for specific internal slots etc.

dcellwanger commented 6 years ago

Thanks, @mtmorgan @LTLA and @drisso . We greatly appreciate your time and your comments.

In my experience, though, it is always worth going the extra mile (within reason) and to add features that improve the usability of software (modules). User-feedback cycles are essential in a good software development process. One should keep in mind, that tools should not only make genomic data accessible to tech-savvy computational biologists, but particularly also to bench scientists. Here, good (rather than quick-and-dirty) data visualizations and simple workflows are tremendously helpful to analyze, understand, and interpret the data. In line with that, I think that accessing genomic feature and sample data with featureNames, featureData, sampleNames, phenoData, usf., as implemented in the ExpressionSet was much more intuitive for a general user, than the more abstract accessor functions (rownames, elementMetadata, colData, usf.) of the SingleCellExperiment. The same holds for the show / print function of this class.

I admit that my decision to extend the ExpressionSet class was likely biased, too, since I was used to store my data in an object of this class from back then, when I was analyzing microarrays. Although it is tempting that I simply extend the SingleCellExperiment class by individual slots, if I go ahead and invest the time to change the architecture of the CellTrails package, I will then make the signatures of the relevant exported functions compatible with a SingleCellExperiment instance. This will allow a smooth integration of CellTrails into a Bioconductor-based SingleCellExperiment analysis workflow. I am currently performing the requirement analysis of the CellTrails framework and will then determine the required adaptions of the package design and its implementation. Certainly, I will need to implement additional accessors and, unfortunately, need to separate the S3 plot method for a CellTrailsSet into single individual functions for a SingleCellExperiment to avoid masking. I see the point that keeping the naming of the assays slot data list elements generic may be of advantage for certain cases, but I do not have a good feeling about the dependency of my functions on a yet undefined element (i.e., name, type). It should be able to represent log-normalized Log2Ex values / counts / intensities. Unfortunately, it seems that this list element is not strictly required and clearly defined in the class. I will stick to the logcounts element, albeit the term counts is not accurate in any case and it is described in the vignette as element containing the log-normalized expression data in most cases.

Please note, that we got word today that the manuscript is accepted. Thus, we need to have a functional package available as soon as the article gets published. I am truly eager to implement the required adaptions and very excited about the opportunity to have CellTrails added to the Bioconductor repository; I hope, though, that the modifications can/will be finished within a reasonable time frame. I would like to avoid having two parallel package versions.

LTLA commented 6 years ago

With respect to making critical changes earlier rather than later; perhaps an anecdote from my past may be helpful. This is in relation to the diffHic package, which I submitted 3-4 years ago. The package reviewer at the time was Nate Hayden, if I recall correctly. He made a number of suggestions that I ignored at the time, because I was hurrying to finish my PhD and just wanted the package to get in. With the benefit of hindsight, I should have listened to him. Here are the consequences:

Using a home-brewed S4 container: I was young and foolish, and thought I would show off my coding flair by writing my own container. This was a Bad Idea, leading to all sorts of maintenance nightmares when my users (i.e., myself) tried to do something slightly unusual, e.g., subsetting and combining. In the end I ended up writing another package (InteractionSet) to implement the container that I needed - based on a SummarizedExperiment, no less. Then I needed to write update methods for my old serialized objects. Then I needed to deprecate my old S4 classes and all of its methods. This work spanned three release cycles (1.5 years), which I could have avoided by doing the right thing at the start.

Irregular unit tests: I put my unit tests in inst/tests where they don't actually get run during the Bioconductor build process. In my defence, this was due to the fact that the tests were really long and would have triggered timeouts (in the age before we had a special build process for long tests). Needless to say, this causes some sleepless nights where I'm not sure that the package is doing as intended on computers other than my own. Another problem was that I wasn't using a formal unit testing system like testthat. If none of my tests failed, then it was fine; but if there was a failure, it was a real pain to track down. I lived in fear of those tests failing after a major refactoring - and still do, because I haven't gotten around to re-writing the tests using testthat, even three years after acceptance.

Static vignette: I have a static PDF vignette for my user's guide, pushed through before it was outlawed in the Bioconductor package guidelines. Again, in my defence, compilation of the vignette takes several hours and involves a number of large data packages, so I couldn't really put it in the package itself. Nonetheless, this means that I need to manually rebuild the vignettes at every release, to check that the contents haven't changed and to ensure that the reported version numbers are up to date. This is a real pain - and now that I've been shut off from my former institute's servers, I can't even do this, because all of the relevant data files are still sitting on those servers.

To put things into perspective; for my popular packages, the number of commits after acceptance into BioC is easily greater than the number of commits before acceptance. Maintenance costs dominate initial development costs, moreso if the initial model was flawed (see my mistakes above). A month's delay to get things right is, quite simply, negligible in the lifetime of a useful package. Of course, for my not-so-popular packages, there's fewer commits involved as fewer bugs are found and features requested, but that's not something to boast about: I don't write packages and expect people not to use them.

Your package will only get better from switching to a SummarizedExperiment. If you need new slots in the SingleCellExperiment, talk to @drisso or me at https://github.com/drisso/SingleCellExperiment. If you play your cards right, you can effectively offload maintenance work to someone else (e.g., see my attempts to do so in the comments in Bioconductor/SummarizedExperiment#10).

Finally, I wouldn't worry about the fact that your manuscript is accepted. You'll be going into BioC-devel, and it'll be 6 months until we get into BioC-release where most of your users will be. There's no rush. In my opinion, for bioinformatics methods, the paper is relatively easy to write, and is effectively just a description/advertisement for the software package. The real struggle is that of maintaining and streamlining the implementation, especially when people start testing (and breaking) it.

bioc-issue-bot commented 6 years ago

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

7b70993 v0.99.3

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: "skipped, ERROR, 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 6 years ago

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

f848552 v0.99.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.

lshep commented 6 years ago

Hi there. I wanted to chime in with a few things:

General: Echoing all the above - it would be better to transition the code now to SingleCellExperiment rather than roll out the package with a less ideal class. Even if the intention is to update it later, this often is either not done or as mentioned than users will already adopt this old class and make the transition even more difficult and disruptive.
Which it looks like you are in the process of so wonderful!!

LICENSE You can remove the LICENSE file as R is distributed with that License it is not needed

DESCRIPTION

NAMESPACE

Vignette:

R Code

bioc-issue-bot commented 6 years ago

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

7369495 v0.99.5

dcellwanger commented 6 years ago

Thanks @Ishep and @LTLA for your time and your comments.

GENERAL I redesigned the package as suggested: it stores all relevant information in an object of class SingleCellExperiment. I agree that integrating CellTrails into a Biconductor-based single-cell analysis pipeline is much more convenient now, albeit multiple packages manipulating the same container and defining functions for the same class may lead to masking issues.

LICENSE Thank you for noting that. I removed the LICENSE file and updated the DESCRIPTION file.

DESCRIPTION I removed the Author/Maintainer and the LazyData entry.

Regarding the dependencies on graphical software and packages. The third-party software is required to properly layout the graph. This is essential for visualization of inferred expression dynamics and to extract single pathways toward a terminal phenotype for further downstream analyses. To our knowledge, there is no proper solution available (in R) for planar weighted embedding of a graph. The procedure within CellTrails is straightforward and denotes only a few steps: write.ygraph exports the graph structure from a SingleCellExperiment object to the common graphml file format, which can be interpreted by the third-party software; the user layouts the graph and saves it, and read.ygraph enables the re-import into the R session. The function trajLayout stores the layout to the SingleCellExperiment object; here, CellTrails adjusts the layout internally, such that distances between data points (samples) represent the latent (pseudo-)temporal distance between adjacent samples. Frankly, we were testing a lot of alternative options, but did not find an ideal solution to avoid the dependency on this particular third-party software. On these grounds, we are open for alternative suggestions.

Please note that the igraph package is not used for visualization, but for graph operations/manipulations. CellTrails imports two (non-standard) packages for visualization:ggplot2 and ggrepel. The former is broadly used and well maintained; it is convenient as it generates ggplot objects, which can be adjusted by the user’s personal preference. The latter package is needed to repel overlapping text labels. I would be grateful if you could point out any redundancies I may have overseen.

NAMESPACE I agree. The most recent version(s) of CellTrails depend on SingleCellExperiment. To avoid the warning triggered by the masking of stats::start and stats::end by import(SummarizedExperiment), I would prefer to import only those function that are actually used – this was also recently suggested by Martin Morgan (@mtmorgan) on the Bioc-Devel mailing list.

VIGNETTE We agree that referring to an external resource is not optimal. However, we really would like to provide a comprehensive and detailed vignette. A CellTrails analysis represents a whole workflow, individual steps of which are dependent on each other (proper checks are implemented). The previous submission/commit v0.99.3 contained the detailed vignette, which caused a server timeout. Building the detailed vignette takes about 2-3 minutes (on my 2015 MacBook) and is about 6.5MB. Not sure whether there is a reasonable solution to overcome the building and file size limitation. Please note that 90% of the code of the detailed vignette is evaluated. Further, the code in the examples is executed and I also added unit tests to prove workable code.

R CODE There won’t be major changes on the source code. I am looking forward to your feedback.

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

bba1657 v0.99.6

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:

a11542a v0.99.7

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

ba3264e v0.99.8

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

dcellwanger commented 6 years ago

Hi @Ishep, I addressed all your comments. The CRAN and BiocCheck seem to be OK on all OS. I look forward to your review report. -Daniel

lshep commented 6 years ago

@dcellwanger I'll try and have a review of the code for you within the next day. Cheers

lshep commented 6 years ago

NEWS

VIGNETTE Running through vignette at https://dcellwanger.github.io/CellTrails/

inst/exdata

Please perform a version bump and comment back here when ready for me to look at again.

bioc-issue-bot commented 6 years ago

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

ecd3817 v0.99.9

dcellwanger commented 6 years ago

@lshep Thank you very much for you comments and the short turnaround time.

NEWS Done.

VIGNETTE • Thanks for noting that. I extended the String printed by showTrajInfo by a new line. • I am wondering about your error there is no package called ‘scran’ - it looks like the package is not installed. I can add the library command to the vignette if strictly required, but generally I try to avoid loading packages unnecessarily. If only one or two specific functions of a comprehensive package are needed, I prefer to call the function directly (e.g., via scran::myFunction). This lowers the odds of name clashes. Also, (on a Mac OS, at least) the number of loaded DLLs is limited (by default) to 100. The package scran loads already 46 DLLs, destiny adds another 39 – with those two packages the limit is almost reached (85 DLLs, including the 6 DLLs loaded by the standard packages). Please note that CellTrails can still be loaded (it adds another 7 DLLs), but the potential of loading packages, which might be more relevant for a user in a specific context, to an R session gets very limited.

inst/exdata Done.

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.

lshep commented 6 years ago

If I was a naive user I may not be aware that all that was needed was a library call but considering you mention scran and destiny in the text section above it, I suppose it is ok. The other option is to use a dontrun{} section to show the library call but not actually have it loaded.

bioc-issue-bot commented 6 years ago

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

552ffda v0.99.10

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.

bioc-issue-bot commented 6 years ago

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

d7d3f62 v0.99.11

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

90710ed v0.99.12

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

dcellwanger commented 6 years ago

@lshep I am sorry for the delay - it looks like the S4Vectors implementation was updated. I was not able to reproduce the error thrown on your server on my local test systems (R 3.5 w/ Bioc devel @ Windows and Mac OS). I assumed that an error got triggered by subsetting a DataFrame object using [[x]] and, indeed, replacing it with [,x] resolved the issue.

The warning which is left indicates a check time exceeding the 5 min requirement (on Windows and OS X). Please let me know if I need to resolve that.

Regarding the vignette. I see your point. The most recent version suggests the library call using a 'dontrun' block in the code snippet; also the text was extended, respectively.

lshep commented 6 years ago

Actually the WARNING is coming from the R version dependency in the DESCRIPTION file:

 * WARNING: Update R version dependency from 3.4 to 3.5.

Which we do recommend updating as this is the version of R that the package will be released under and what we are testing on our systems.

dcellwanger commented 6 years ago

Thanks for the note. I guess you refer to the build report of v0.99.11. The DESCRIPTION file of v0.99.12, however, should contain 'Depends: R (>= 3.5)'. The WARNING you mentioned is not stated in the corresponding (latest) build report. Please let me know if a version bump is needed.

lshep commented 6 years ago

Ah sorry I was looking at a previous report - The S4Vectors has been resolved as well I believe - But I will have a glance at your package again within the next day or two for feedback.