Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

TVTB: The VCF Tool Box #111

Closed kevinrue closed 8 years ago

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

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: TVTB
Type: Package
Title: TVTB: The VCF Tool Box
Version: 0.99.0
Date: 2016-09-15
Authors@R: person(
    "Kevin", "Rue-Albrecht", role = c("aut", "cre"),
    email = "kevinrue67@gmail.com")
Description: The package provides S4 classes and methods to filter,
    summarise and visualise genetic variation data stored in VCF files.
    In particular, the package extends the FilterRules class (S4Vectors
    package) to define news classes of filter rules applicable to the various
    slots of VCF objects.
    Functionalities are integrated and demonstrated in a Shiny web-application,
    the Shiny Variant Explorer (tSVE).
License: Artistic-2.0
Depends:
    R (>= 3.3), methods, utils, stats
Imports:
    BiocGenerics (>= 0.19.1),
    BiocParallel,
    Biostrings,
    ensembldb,
    ensemblVEP,
    GenomeInfoDb,
    GenomicRanges,
    ggplot2,
    IRanges (>= 2.7.1),
    reshape2,
    Rsamtools,
    S4Vectors (>= 0.11.11),
    SummarizedExperiment,
    VariantAnnotation (>= 1.19.9)
Suggests:
    EnsDb.Hsapiens.v75 (>= 0.99.7),
    shiny (>= 0.13.2.9005),
    DT (>= 0.1.67),
    rtracklayer,
    BiocStyle (>= 2.1.23),
    knitr (>= 1.12),
    rmarkdown,
    testthat,
    covr
biocViews:
    Software,
    Genetics,
    GeneticVariability,
    GenomicVariation,
    DataRepresentation,
    GUI,
    Genetics,
    DNASeq,
    WholeGenome,
    Visualization,
    MultipleComparison,
    DataImport,
    VariantAnnotation,
    Sequencing,
    Coverage,
    Alignment,
    SequenceMatching
Collates:
    ensembldbFunctions.R,
    regions2vcf.R,
    runShinyApp.R,
    AllClasses.R,
    AllGenerics.R,
    TVTBparam-class.R,
    VcfFilterRules-class.R,
    countGenos-methods.R,
    addCountGenos-methods.R,
    addFrequencies-methods.R,
    addOverallFrequencies-methods.R,
    addPhenoLevelFrequencies-methods.R,
    dropInfo.R,
    variantsInSamples-methods.R,
    vepInPhenoLevel-methods.R,
    tabulateVep-methods.R,
    densityVep-methods.R,
    show-methods.R
VignetteBuilder: knitr
URL: https://github.com/kevinrue/TVTB
BugReports: https://github.com/kevinrue/TVTB/issues
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: "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/TVTB_buildreport_20160915132510.html

kevinrue commented 8 years ago

Dear all,

I'm not sure whether there is a way to avoid the nowidow.sty LATEX package or if moscato1 needs a little update.

Note that I had a look at the *tex document and I cannot see any occurence of "nowidow" in it.

Kind regards, Kevin

mtmorgan commented 8 years ago

Hi Kevin -- probably it is an indirect LaTeX dependency; we updated the build server to have the package. I'll review it as-is.

lawremi commented 8 years ago

This looks like an important contribution, and I'd like to provide constructive feedback. Would it be best to post them as issues on the source repository, or here? I guess I will post issues and reference them from here, if that's even possible. What's the best practice here?

mtmorgan commented 8 years ago

It is helpful to have your comments included in this issue, so at least a summary or link to issues / pull requests made elsewhere is appropriate.

kevinrue commented 8 years ago

Hi Michael,

I agree with Martin. Moreover, this is also what I expected from reading the submission guidelines1, and that I hoped for by submitting the package now, before moving on to even more ambitious features.

1Other Bioconductor developers and users with domain expertise are encouraged to provide additional community commentary. Reviewers will add comments to the issue you created.

Even if it means "missing" the next release (interested users could always get it from the devel branch), I am happy to take onboard all the constructive feedback that I can get to validate the new concepts I wish to contribute, or rectify my trajectory where necessary; I have particularly in mind the VCF filter rules and to a lesser extent the more experimental TVTBparam class.

Thanks for your interest!

lawremi commented 8 years ago

I find these linear threads to be tough to follow when discussion becomes complex.

But, just to get thing started, I find the explicit argument naming in the method and class definitions to impinge on readability. Perhaps a matter of personal taste/habit. But it's tough for me to read.

You might not need to define so many initialize() methods. Combined with the class prototype, the default method usually works. It's such a low-level generic, we usually leave it alone and instead define high-level constructor functions.

It also seems you could make better use of dispatch to handle the polymorphic behaviors of the different filter types. E.g., you could avoid stuff like switch() statements. It's a more extensible approach that might also clarify the code.

kevinrue commented 8 years ago

I see all your points.

Regarding the linear thread, I think it's good to mention here for starters. We can always open issues for fixes or enhancements that require a more in-depth discussion.

I can see all your points, but I'll need a little more details to focus my updates:

  1. argument naming: could you please list the ones that you have in mind?
  2. initialize methods: I'll look back into that using some other BioC packages as template. To be honest, this is the first time I implemented S4 classes following online tutorials and I might have gone over the top following them blindly in some places. I can't find right now where I got the initialize concept from, but I'm happy to remove it if that's not standard practice.
  3. I see the switch statements that you mean, and I think I can see how to replace them by a method dispatch.

So please just clarify point 1, and keep the feedback coming; I'm happy to feel some interest :) Best wishes, Kevin

lawremi commented 8 years ago

I just mean that naming every argument to setMethod() makes it hard to parse. That might just be me being used to the standard syntax for method definition.

kevinrue commented 8 years ago

So we're talking about changing:

setMethod(
    f = "addCountGenos",
    signature = c(vcf="ExpandedVCF"),
    definition = function(
        vcf, genos, key, description, samples = 1:ncol(vcf), force = FALSE){
        <code>
        }

into:

setMethod(
    "addCountGenos", c("ExpandedVCF"),
    function(
        vcf, genos, key, description, samples = 1:ncol(vcf), force = FALSE){
        <code>
        }

Right? (Just making sure before I update the whole package on a misunderstanding :grin: )

Spelling out the definition of S4 methods and classes was a bit clearer to me while learning the meaning of each argument, but I think I got familiar enough now to move on to the latter syntax, which seems to be the Bioconductor standard as far as I can see (S4Vectors, VariantAnnotation, ...).

Cheers!

lawremi commented 8 years ago

Yea, usually it's like:

setMethod("addCountGenos", "ExpandedVCF", function(...) {
})
kevinrue commented 8 years ago

HI Michael,

I've created the branch BiocReview to work on your comments. So far, I've dealt with the setMethod() point, if you prefer to checkout the code from that branch for reviewing purpose.

Going through the code for the above point, I have identified some code cleaning that will make it easier to read and more organised before dealing with your other two points (I call them initialize and switch)

All the best, Kevin

kevinrue commented 8 years ago

Hi @lawremi ,

It also seems you could make better use of dispatch to handle the polymorphic behaviors of the different filter types. E.g., you could avoid stuff like switch() statements. It's a more extensible approach that might also clarify the code.

I have replaced two of the four switch statements in VcfFilterRules-class.R, by implementing accessors that return NA_character_ for filter classes that do not have the relevant slot to remain consistent with the return type and length (e.g. VcfFixedRules do not have vep and type slot).

Looking back at the switch statement in [<- for signature VcfFilterRules, I realise that I wrote those complex methods to override the behaviour of the parent method (inherited from List), which does not transfer the name and active data of rules (it does transfer the elementMetadata though). I sent an email to maintainer at bioconductor to discuss this outside this linear thread.

I think my mechanism could be greatly simplified by the following

In any case, I need to re-design my filter classes, and their subsetting/replacement methods, all depending on the parent FilterRules class and associated methods.

Thanks in advance for advising, Kevin

lawremi commented 8 years ago

I wouldn't use any of the metadata components to support a formal representation. You can set a method on the parallelSlotNames() generic to handle your type slot.

kevinrue commented 8 years ago

Hi @lawremi ,

Just to let you know that following your advice, I considerably trimmed down my code.

Thanks for the constructive feedback! It is great to see my code being "simplified" by those changes. Also, revisiting my code, I noticed some issues, including if blocks that span multiple lines without being enclosed in {}. I'm fixing that while awaiting Martin's review (I assume based on the version of the code that I originally submitted).

EDIT 1: I just realised that you also mentioned "explicit argument naming" in methods definitions. I missed that point and only dealt with it in class definitions. Fixing that now. EDIT 2: Fixed EDIT 1. I started applying it to unit tests as well, with lower priority. I think this time I have fully dealt with your first batch of suggestions. Moving on to the second one (below).

lawremi commented 8 years ago

Thanks for these efforts. It's exciting that we will soon gain a set of high quality tools sitting downstream of VEP-based workflows.

Here are some more suggestions:

  1. Would it make sense to more formally define the genos slot of TVTBparam?
  2. In the introduction vignette, when importing the phenotypes, you can pass multiple path tokens to system.file(), so there is no need for a separate call to file.path().
  3. Would it make sense to define a coercion, via setAs(), to generate a ScanVcfParam from a TVTBparam ? And maybe add a readVcf() method on TVTBparam for convenience? That method might also accept the phenotype information.
  4. I really like how you have factored all of the options into the TVTBparam class. What attaching the TVTBparam to the VCF object returned from your (potential) readVcf() wrapper? You could use the metadata(). Then, the user would not need to pass the param to downstream functions.
  5. There are some summary functions that have a plot argument for generating a plot. It would be cleaner if the summary was separated from the plot, so I could generate a summary and later plot it. Perhaps a high-level object could be returned that has a method on the ggplot2 autoplot() generic.
kevinrue commented 8 years ago

Hi Michael,

Thanks for the encouragement. I really appreciate a community feedback to distinguish generally useful features from those only appealing to me.

I am still going through the methods to get rid of superfluous explicit argument naming. But I'd appreciate if you could clarify (maybe edit your post instead of posting another one?):

(1) Do you mean an S4 class dedicated for this purpose? Now that I think of it, it would probably be clearer/cleaner to have a dedicated validity method, rather than including this in the TVTBparam validity method.

(3) Interesting thought. I used to have a preprocessVcf method wrapped around readVcf which was handling VCF file and phenotype, and ended up with exponential increase of the number of signatures (phenotypes={data.frame, DataFrame,missing}, vcf={character,TabixFile}). I gave up the idea thinking that users may prefer to handle the few lines of code to import and combine data. It might be worth revisiting the idea after I finish my current cleaning.

(4) It did cross my mind. I think I was just shy about touching VCF objects. Worth revisiting after point (3) above.

(5) Agreed. I have recently banged my head against the wall figuring out ways to make the *ByPhenotype methods cleanly use loops around their associated *InPhenotype counterpart. Moving the plot argument out of the equation would definitely simplify things by avoiding unnecessary if-else blocks. I didn't think of the autoplot method!

lawremi commented 8 years ago

Yea, I meant an S4 class for (1). For (3), you might not need so many top-level signatures. Instead, your implementation could be abstracted by dispatch through lower level generics.

kevinrue commented 8 years ago

If that's okay, I'll use (update) this post to reflect progress on your five suggestions, now that I'm pretty clear about them.

  1. ✅ Formally define the genos slot of TVTBparam
    • ✅ I have implemented the S4 class Genotypes to store... genotype definitions.
    • ✅ I have replaced my superfluous hRef and hAlt accessors, by ref and alt using the generic already defined in VariantAnnotation.
    • ✅ I will update TVTBparam to store an object of class Genotypes in its genos slot.
  2. ✅ Done.
    • Smarter call to system.file
  3. ✅ Pending 1. Somewhat affected by genotype storage structure
    • setAs() ScanVcfParam from a TVTBparam
    • readVcf() method on TVTBparam for convenience? Support optional phenotype information.
  4. ✅ Pending 3. Affected by updates to TVTBparam class
    • ✅ Attach TVTBparam to the VCF object returned from the above readVcf() wrapper. Could use the metadata slot.
    • ✅ Removes requirement for param argument in downstream functions.
  5. 💢 Pending 4. Affected by updates to TVTBparam storage location
    • 💢 Remove summarisation/storage methods. The feature may be revisited in the future; however, user code may be more efficient/flexible than convoluted multi-purpose wrapper methods.

Better late than never: ✅ declared type as a parallelSlotNames to simplify subsetting and replacement ✅ defined setAs method instead of coerce for Vcf*Rules

Notes:

All the best, Kevin

mtmorgan commented 8 years ago

Hi Kevin -- the formal deadline for adding packages to the forthcoming release of Bioconductor is Wednesday, October 12; are you able to complete your modifications soon? Thanks for your work on the package.

kevinrue commented 8 years ago

Dear Martin,

Thanks for your e-mail. I am fairly sure that I won't make this deadline. Unfortunately, I had limited time recently, as this is a personal endeavour, complementary to my postdoctoral duties, yet not encouraged during my working hours for the benefit of other duties.

I'm happy to make the code available in the next devel branch, it will give me more time to polish the code and docs.

Note that in the next (big) commit the families of functions tabulate* and density* will be removed (the code will be stored in a "sandbox" subfolder until the feature is made available again) as I agree with Michael: I need to separate summarization from plotting. With that in mind, I actually started wondering whether it would actually be simpler to leave some of the summarization to the user, rather than developing a complex multi-purpose wrapper method.

I'll post again here when after the next commit for a clearer update.

Cheers Kevin

On 9 Oct 2016 02:53, "Martin Morgan" notifications@github.com wrote:

Hi Kevin -- the formal deadline for adding packages to the forthcoming release of Bioconductor is Wednesday, October 12 http://bioconductor.org/developers/release-schedule/; are you able to complete your modifications soon? Thanks for your work on the package.

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

mtmorgan commented 8 years ago

OK thanks for the update Kevin and for your work on this package.

bioc-issue-bot commented 8 years ago

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

eef954d Standardised S4 method definition: unnamed positio... 057f515 Previously missed: Standardised S4 method definiti... eb5d772 Merge branch 'BiocReview' of github.com:kevinrue/t... b260809 More code sections and comments; add line breaks t... c9265d7 Use "pkg::" prefix in unit tests to run them witho... 4c20fe9 Replaced one unnecessary switch statement; impleme... 42ebe64 Use vep accessor dispatch to any VCF filter rule r... f026819 Implemented type accessor for VCF fixed/info/vep r... a1ecb2b Minor comment clarification db7c6b6 Removed initialize methods for VcfFixedRules/VcfI... 97717ce Merge branch 'BiocReview' of github.com:kevinrue/T... 8d67659 Udpated man pages to document recently added acces... 44ebca1 typos in doc 622931e use @ internally instead of accessor 1489c58 Remove initialize VcfVepRules; move code to the de... b69b1ae internally use @ instead of slot method for validi... 89bb2b3 Removed initialize VcfFilterRules; moved coded to ... 94a48e8 Removed last initialize TVTVparam, S4 constructor ... c59be97 Correction of man page. e11d12a Simplified subsetting of VcfFilterRules; unit test... 0da247b Rule types are shown as named list (like the activ... 92b17a6 use parent method (FilterRules, List, Vector) for ... c28e3da Minor comment update 6f4bc43 added return statement for clarity f3944a8 Added return statement for clarity; removed redund... ee9c447 added return statements for clarity; removed unnec... 6af0f2f Minor comment updates 0ac7d65 Added return statements for clarity; removed unnec... eb2d4e0 Added return statements for clarity; removed unnec... e84d231 Removed unnecessary explicit argument naming. 330f960 replaced if-else by stopifnot; do not check VCF fi... 817e1a1 Removed superflous explicit argument naming. b0a447a Better use of system.file d3ed103 bug fix in Introduction.Rmd system.file 06fb64c removed unnecessary explicit argument naming; encl... d4f4645 Enclosed if statements in {}; keep current names i... 79ffa84 typo bug fix. 0f5b01f enclosed if blocks in {}; added TVTBparam validity... f9cf75f minor comment 9879e81 enclosed more if blocks in {}; rules do not requir... 30db80e enclosed more if blocks in {}; additional validity... 6c083be Merge branch 'BiocReview' of https://github.com/ke... 00c8eb8 Restored 100% coverage due to unncessary if-else. 3302d55 removed superfluous explicit argument naming; smar... 88038df removed superfluous explicit argument naming; encl... 280e313 Attempt to trigger Travis for BiocReview branch ca77993 typo in the test. a0b90ee added return statements for clarity; validation of... 5cea783 Initial work for the Genotypes class: class defini... 998f92f export new ref and alt accessors/setters defined f... ff7a468 Removed superfluous hRef and hAlt methods for acce... 5482264 Solved a superfluous russian-dolls if block 7e10a07 Placed Genotypes-class.R in collate order; removed... 96f8c8a do not display na.omit in vep slot; declare type a... ff5bb3f warning message for tSVE during major package upda... 8537300 default values declared in VcfFilterRules class de... f60cb8e setAs method from TVTBaram to ScanVcfParam 3ce7ede setAs does not set geno slot of ScanVcfParam (load... cfbb1a4 setAs does not set the info slot of scanVcfParam u... 6102b52 added readVcf signatures for TVTBparam; reduce ran... c10e299 readVcf signatures for TVTBparam attach TVTBparm a... 9fcbf05 TVTBparam class new slot for ScanVcfParam; TVTBpar... 513c44b coding style typo ca357a5 renamed tSVE script to match function name; moved ... cf42c87 updates to the Introduction vignette f7a131a typo 235bc0b VcfFilterRules can include FilterRules that are ap... 2e06063 TVTBparam removed from arguments in downstream met... ea6149b Version bump; NEWS file updated. b4b83df Merge pull request #3 from kevinrue/BiocReview Bi...

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: "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/TVTB_buildreport_20161011050107.html

kevinrue commented 8 years ago

Hi @mtmorgan ,

Actually, if that's okay with you, I have merged the BiocReview branch of my repository to the master branch built by the tracker. I have now reacted to all of @lawremi 's suggestions, and trimmed out unstable/unnecessary methods (see below). I believe the current features may actually be helpful to release already, while I work on the devel branch to experiment and develop more features.

A couple of considerations:

Kind regards, Kevin

mtmorgan commented 8 years ago

This type of error seems to occur when the processor gets ahead of the file system, for instance because large files (e.g., vector representations of millions of points, rather than reasonable-sized bitmaps) are being created. I don't know if that offers any hints? We could / will 'let it go' now, but these tend to re-appear during the regular builds.

kevinrue commented 8 years ago

Actually, it may be unrelated, but I just realise now that the warning messages displayed in the errored build should not be there at all (my methods are designed to avoid raising those VariantAnnotation warnings). I'm homing in on the source of that problem right now, ensuing from recent updates as part of the review process, and I should be able to trigger another build in a couple of hours.

I do not see a link between those warnings and the build error, but maybe there is one. Otherwise, if it crops up again in future builds, I'll be happy to investigate in more detail. It never happened to me so far (Ubuntu & OSX machines tested)

I am only using a small VCF file as input (150kb *.vcf.gz file; 1.3 MB VCF object in memory during the R session; 481 variants found within a single gene coding region). Do you see any obvious issue in this? I don't see where I would be generating millions of points.

bioc-issue-bot commented 8 years ago

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

a021dbe Version bump; Bug fix: use suffix accessor in add*...

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: "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/TVTB_buildreport_20161011121010.html

kevinrue commented 8 years ago

Hi again @mtmorgan ,

Hm... The messages that appear in the new failed build are now exactly the ones expected.

However, as I suspected, that issue was distinct from the source of the failed build on moscato1. It's a shame I haven't yet a Windows machine set up to build R packages.

Is there any chance that using BiocParallel (even with SerialParam) causes any issue on Windows? I have come across situations where some OSes didn't like certain parallel settings. I remember seeing Windows mentioned somewhere about that.

Thanks for advising Kevin

bioc-issue-bot commented 8 years ago

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

f22baaf disabled BiocParallel code to experiment with the ...

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: "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/TVTB_buildreport_20161012041900.html

mtmorgan commented 8 years ago

@kevinrue yes, open connections have caused problems in the past, which can happen when a 'worker' is either not closed (bpstart() / bpstop() not balanced; if you do not explicitly call these then not relevant) or a worker dies but is not cleaned up (e.g., because the worker dies but is itself holding on to some other connection, like an open file). The worker dying would be revealed by SerialParam(). I'll try later today to debug this.

bioc-issue-bot commented 8 years ago

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

ff70dd1 Reenabled the BiocParallel code as the previous co...

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: "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/TVTB_buildreport_20161012053212.html

bioc-issue-bot commented 8 years ago

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

29750cf fixed Collate: field of DESCRIPTION file.

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: "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/TVTB_buildreport_20161012104456.html

kevinrue commented 8 years ago

Hi @mtmorgan (@aoles might be interested in this too)

I have come across something strange, which might be related to my Windows build problem):

After setting up a virtual machine running Windows 10, Rtools.exe, Perl, MikTek 2.9, ...

BiocStyle::pdf_document() works:

render("Introduction.Rmd", BiocStyle::pdf_document(), intermediates_dir = "tmp", clean = FALSE)

pdf_document2() (which I have used so far) does not work:

render("Introduction.Rmd", BiocStyle::pdf_document2(), intermediates_dir = "tmp", clean = FALSE)

with message (after successfully evaluating all the R code blocks)

...
output file: C:/Users/Rue-AlbrechtKevinC/Documents/git/TVTB/vignettes/tmp/Introduction.knit.md

"C:/Program Files/RStudio/bin/pandoc/pandoc" +RTS -K512m -RTS "C:/Users/Rue-AlbrechtKevinC/Documents/git/TVTB/vignettes/tmp/Introduction.utf8.md" --to latex --from markdown+autolink_bare_uris+ascii_identifiers+tex_math_single_backslash --output pandoc1d3837299.tex --table-of-contents --toc-depth 2 --template "C:\Users\RUE-AL~1\AppData\Local\Temp\RtmpqWOvZ1\BiocStyle\template.tex" --number-sections --highlight-style tango --latex-engine pdflatex --include-in-header "C:\Users\RUE-AL~1\AppData\Local\Temp\RtmpqWOvZ1\1d385ec8499.tex" --bibliography TVTB.bib --filter pandoc-citeproc 
Error: Failed to compile Introduction.tex.
In addition: Warning messages:
1: running command '"C:\Users\RUE-AL~1\AppData\Local\Programs\MIKTEX~1.9\miktex\bin\x64\latexmk.exe" -pdf -latexoption=-halt-on-error -interaction=batchmode -pdflatex="pdflatex" "Introduction.tex"' had status 1 
2: running command '"C:\Users\RUE-AL~1\AppData\Local\Programs\MIKTEX~1.9\miktex\bin\x64\latexmk.exe" -v' had status 1 

To test my working hypothesis, I thing I will commit a new release after changing the YAML header from:

output:
    BiocStyle::pdf_document2:

to

output:
    BiocStyle::pdf_document:

to see if that solves anything. Otherwise, I'll just revert to pdf_document2

All the best, Kevin

bioc-issue-bot commented 8 years ago

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

70a403d Replaced pdf_document2 by pdf_document in YAML he...

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/TVTB_buildreport_20161012122023.html

bioc-issue-bot commented 8 years ago

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

902a820 NEWS fixed closing brackets

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/TVTB_buildreport_20161012123359.html

kevinrue commented 8 years ago

Tadaaaaaaa! 🎉 🎈 🎂

@aoles and @mtmorgan

The build error is now only caused by an independent minor issue: a timeout about checking bioc-devel mailing list subscription. Regarding the Windows build error that I was tracking down, it seems to be be caused by a bad interaction between the BiocStyle::pdf-document2() style and Windows. In contrast, pdf-document() or html_document/2 succesfully build the vignettes on Windows (no problem to report on Linux or OSX).

However, that only means that I know how to avoid the problem, not how to solve it.

All the best, Kevin

bioc-issue-bot commented 8 years ago

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

e2c38a7 Set vignette output format to BiocStyle::html_docu...

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/TVTB_buildreport_20161012132732.html

mtmorgan commented 8 years ago

Hi @kevinrue the package looks great and I'll mark it as accepted. It will be added to the nightly build system and svn, with additional information provided by email in the next several days. It will be included in the forthcoming Bioconductor release, even if accepted slightly after the deadline. Thanks for your work!

kevinrue commented 8 years ago

Thanks @mtmorgan amd @lawremi

All the best, Kevin