Submission: mcbette

[richelbilderbeek]

Submitting Author: Richel Bilderbeek (@richelbilderbeek)
Repository: https://github.com/richelbilderbeek/mcbette
Version submitted: 1.8.2
Editor: @maelle
Reviewer 1: @vbaliga
Reviewer 2: @bjoelle
Archive: TBD
Version accepted: TBD

---

• Paste the full DESCRIPTION file inside a code block below:

Package: mcbette
Title: Model Comparison Using 'babette'
Version: 1.8.2
Authors@R: c(
    person("Richel J.C.", "Bilderbeek", email = "richel@richelbilderbeek.nl", role = c("aut", "cre"), comment = c(ORCID = "0000-0003-1107-7049")))
Maintainer: Richel J.C. Bilderbeek <richel@richelbilderbeek.nl>
Description: 'BEAST2' (<http://www.beast2.org>) is a widely used
    Bayesian phylogenetic tool, that uses DNA/RNA/protein data
    and many model priors to create a posterior of jointly estimated 
    phylogenies and parameters.
    'mcbette' allows to do a Bayesian model comparison over some
    site and clock models, 
    using 'babette' (<https://www.github.com/ropensci/babette>).
License: GPL-3
LazyData: true
RoxygenNote: 7.0.2
VignetteBuilder: knitr
URL: https://github.com/richelbilderbeek/mcbette
BugReports: https://github.com/richelbilderbeek/mcbette/issues
Depends:
    babette,
    beautier (>= 2.3),
    beastier (>= 2.0.25),
    mauricer,
    tracerer
Imports:
    knitr,
    Rmpfr,
    testit
Suggests:
    ape,
    devtools,
    ggplot2,
    hunspell,
    lintr,
    nLTT,
    phangorn,
    rappdirs,
    rmarkdown,
    spelling,
    testthat (>= 2.1.0)
Language: en-US
Encoding: UTF-8
SystemRequirements: BEAST2 (http://www.beast2.org/)

Scope

• Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

  • data retrieval
  • data extraction
  • database access
  • data munging
  • data deposition
  • workflow automation
  • version control
  • citation management and bibliometrics
  • scientific software wrappers
  • database software bindings
  • geospatial data
  • text analysis

• Explain how and why the package falls under these categories (briefly, 1-2 sentences):

mcbette automates a model comparison using the tool 'BEAST2' with one of its packages.

⚠️ because mcbette is a wrapper for 'BEAST2' and because 'BEAST2' does not allow to do what mcbette does on Windows, this package does not work under the Windows operating system.

• Who is the target audience and what are scientific applications of this package?

This package aims at scientists in the field of phylogenetics, as it allows to do a Bayesian model comparison of phylogenetic inference models. Or simpler: with mcbette you can pick the best model to build a phylogeny from a DNA sequence.

• Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?

There is some overlap, as model comparison is an important topic in phylogenetics. Unique about mcbette is the way how it does so: it uses a novel 'BEAST2' package (called 'NS', shorthand of 'Nested Sampling') to directly estimate the marginal likelihood (a.k.a. the evidence) of an inference model. Because the technique of Nested Sampling is -in this field- novel, the 'BEAST2' package is novel and mcbette is novel.

• If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Technical checks

Confirm each of the following by checking the box. This package:

• does not violate the Terms of Service of any service it interacts with.
• has a CRAN and OSI accepted license.
• contains a README with instructions for installing the development version.
• includes documentation with examples for all functions.
• contains a vignette with examples of its essential functions and uses.
• has a test suite.
• has continuous integration, including reporting of test coverage using services such as Travis CI, Coveralls and/or CodeCov.

Publication options

• Do you intend for this package to go on CRAN?
• Do you wish to automatically submit to the Journal of Open Source Software? If so:

JOSS Options

• The package has an obvious research application according to JOSS's definition.
  • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.

I do intend to submit to JOSS, as it looks awesome! Because I did not expect this (for me, new) option, I am not prepared yet. I will await the acceptance of my submission first, than happily write this!

- [ ] The package is deposited in a long-term repository with the DOI: 

Not yet.

- (*Do not submit your package separately to JOSS*)  

• Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

MEE Options

• The package is novel and will be of interest to the broad readership of the journal.
• The manuscript describing the package is no longer than 3000 words.
• You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
• (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
• (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
• (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

• I agree to abide by rOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

[maelle]

Editor checks:

• Fit: The package meets criteria for fit and overlap
• Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
• License: The package has a CRAN or OSI accepted license
• Repository: The repository link resolves correctly
• Archive (JOSS only, may be post-review): The repository DOI resolves correctly
• Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

---

Editor comments

Thanks @richelbilderbeek for your submission! I'm now looking for reviewers.

In the meantime, I have a few comments/questions you can address:

• The link to BEAST2 in DESCRIPTION uses http, please replace it with https <https://www.beast2.org/> (I made a PR). If you used the http link in other packages, you can change it but that is obviously beyond the scope of my checks. 😉

• Have you thought about providing guidance/examples on other CI services e.g. GH actions, Circle CI?

• In the README I think the existence of two modes (online, and offline on Mac and Linux) could be made clearer.

• I made a PR for replacing usethis with remotes in the installation guidelines, which you've merged.

• In the README section about installation, please write how many dependencies in total are needed by the package (I had to install >30 packages) and that rJava and Rmpfr are among them (on e.g. Ubuntu, it means installing system dependencies before installing the R packages),
  to make it less of a surprise.

• In the FAQ, it says the other packages are not on CRAN, but they're on CRAN now, aren't they?

• Why is there no README.Rmd?

• Are your R packages listed somewhere on BEAST2 website?

• I note the JOSS paper.md isn't there yet.

goodpractice output

  ✖ not use "Depends" in DESCRIPTION, as it can cause name clashes, and
    poor interaction with other packages. Use "Imports" instead.
  ✖ not import packages as a whole, as this can cause name clashes between
    the imported packages. Instead, import only the specific functions you need.

What are the reasons for importing the whole packages?

  ✖ fix this R CMD check WARNING: LaTeX errors when creating PDF version.
    This typically indicates Rd problems. LaTeX errors found: ! LaTeX Error: File
    `ltxcmds.sty' not found. Type X to quit or <RETURN> to proceed, or enter new
    name. (Default extension: sty) ! Emergency stop. <read *> l.105
    \RequirePackage{ltxcmds}[2010/11/12] ^^M !  ==> Fatal error occurred, no output
    PDF file produced!

Maybe/probably just a setup thing?

---

Reviewers: @vbaliga @bjoelle
Due date: 2020-03-30

[maelle]

You can now add a review badge rodev::use_review_badge(360).

[maelle]

I forgot a question: why are there expectations in examples? Can't these cases & expectations live in tests?

[maelle]

Reviewer 1: @vbaliga (thanks for agreeing to review!). I'll add a deadline once the second reviewer is assigned.

[richelbilderbeek]

Thanks @maelle for the first wave of feedback 👍

The link to BEAST2 in DESCRIPTION uses http, please replace it with https https://www.beast2.org/ (I made a PR). If you used the http link in other packages, you can change it but that is obviously beyond the scope of my checks. wink

• Will do here

Have you thought about providing guidance/examples on other CI services e.g. GH actions, Circle CI?

I have thought about about it, but I felt it not worth the effort to put another CI in place. Usually I add an AppVeyor build to check on Windows, but because mcbette cannot work on Windows (due to BEAST2), I skipped the AppVeyor build (although I could add a trivial build).

You think I should?

• In the README I think the existence of two modes (online, and offline on Mac and Linux) could be made clearer.

• Will do here

• I made a PR for replacing usethis with remotes in the installation guidelines, which you've merged.

Thanks!

• In the README section about installation, please write how many dependencies in total are needed by the package (I had to install >30 packages) and that rJava and Rmpfr are among them (on e.g. Ubuntu, it means installing system dependencies before installing the R packages),
  to make it less of a surprise.

• Will do so here

In the FAQ, it says the other packages are not on CRAN, but they're on CRAN now, aren't they?

They are, I will update the FAQ

• Update FAQ here

Why is there no README.Rmd?

Because my other R packages on rOpenSci do not have it either. I'll look into this.

• Look into this here

Are your R packages listed somewhere on BEAST2 website?

No. BEAST2 has its own (albeit BEAST2) packages. I think they are aware of babette. You think I should contact them?

I note the JOSS paper.md isn't there yet.

Correct, I made an Issue here. This JOSS item on rOpenSci is new to me, and I think it's cool. Because I am in the lost months of my PhD, though, I do need to be careful with my time. I suggest the deadline of March 2nd February 9th. Would that work?

goodpractice output

✖ not use "Depends" in DESCRIPTION, as it can cause name clashes, and
poor interaction with other packages. Use "Imports" instead.
✖ not import packages as a whole, as this can cause name clashes between
the imported packages. Instead, import only the specific functions you need.

What are the reasons for importing the whole packages?

There are -really!- dozens of functions that need to be imported. OTOH, goodpractice is right! I fixed it

✖ fix this R CMD check WARNING: LaTeX errors when creating PDF version.
This typically indicates Rd problems. LaTeX errors found: ! LaTeX Error: File
`ltxcmds.sty' not found. Type X to quit or to proceed, or enter new
name. (Default extension: sty) ! Emergency stop. <read *> l.105
\RequirePackage{ltxcmds}[2010/11/12] ^^M ! ==> Fatal error occurred, no output
PDF file produced!

Maybe/probably just a setup thing?

Thanks, I have fiixed it

I forgot a question: why are there expectations in examples? Can't these cases & expectations live in tests?

After asking the CRAN maintainers for help, indeed, one should not put tests in examples. I've removed these.

[maelle]

Thank you!

No need to add a trivial Appveyor build. :-)

Yes I think it might make sense to contact BEAST2 developers because their website is probably the first place an user would look for an R package? (second being a search engine, which would probably uncover your packages).

Yes let's put this on hold until 2020/02/09 (@vbaliga, thanks for your understanding, sorry for contacting you too soon). Good luck with your PhD tasks!

[richelbilderbeek]

Thanks @maelle 👍

[maelle]

Second reviewer @bjoelle (thank you!) - after 2020/02/09

[richelbilderbeek]

@maelle: the JOSS paper is ready for review, but I still need some time (say, another weekend) to fix the other points you've mentioned.

[maelle]

Great, thanks for the update!

[richelbilderbeek]

@maelle: I've worked on some of the points you've mentioned, but have not been able to complete them all yet. Will probably fix before Sunday February 16th 23:59, or at least give another update :+1;

[richelbilderbeek]

Just a quick update: done everything, except that I am struggling with some goodpractice issues.

• I did add a AppVeyor/Windows build, just to be sure the package can build under Windows.
• I added a README.Rmd as requested. It's a cool and useful feature!
• JOSS paper is ready for review

Will post when having solved those last things. Then -AFAICS- the package is ready for review 👍

[maelle]

thanks for the update!

[richelbilderbeek]

Just a quick update: nearly there, just waiting for the Travis CI and AppVeyor builds to finish, but this will take until tomorrow (I've queued quite some). I will update when I did the last tiny tidbits 👍

[richelbilderbeek]

Still to do:

• contact BEAST2 developers
• fix noise in Travis logs generated by goodpractice

[richelbilderbeek]

The first reviewer's comments:

Installation

I had trouble with the package installation due to issues with rJava, and never got it working on my Mac (macOS 10.14.6). The CRAN version of rJava is looking for a specific JDK which isn't the one on my machine. Compiling it from source appeared to work, but running any function crashes the R console. Installing on a Linux cluster worked.

It looks to me as if the dependency on rJava is only used in the get_default_java_path function, which seems disproportionate with the amount of trouble it is. I would recommend exploring whether the dependency can be removed, and if not providing a work-around for people who cannot get rJava working - one option would be to ask users to provide the path to Java manually.

I completely agree that getting rJava to work is the toughest nut to crack! I've seen this is true for Linux, Mac and Windows. Yet, beastier (a dependency of mcbette) needs it to -indeed- get the default Java path as well getting the Java version, which helps out in processing bug reports (created by beastier::beastier_report).

However, I feel hand-coding the same subset of functionality is not the way to go. rJava is the package to work with Java and I hope (and predict) the rJava maintainers to simplify its installation. I feel the responsibility about get Java info is at rJava, instead of mcbette: mcbette is there for model comparison.

Note that the rJava maintainer strongly recommends upgrading to rJava 0.9-12 at the 2020/03/23 rJava release update: maybe that update fixed exactly the problem Joelle had.

Other comments

instructions on how to configure to use an existing BEAST2 installation would be helpful, if it is possible.

I have documented using BEAST2 installed at a custom location with this commit.

On Mac mcbette is not looking for it in the default place it is installed to, so that could lead to many duplicate installations.

This would indeed be weird! I looked into this, by writing a test that installs BEAST2 at a custom location, after which mcbette is called to use that BEAST2 install (see this commit). The test, however, passes. If this weird behavior could be reproduced, that would definitely be hulpful, but up until then, I cannot fix a problem I cannot reproduce.

Documentation

the documentation (paper, README, vignette) was generally unclear as to which models can be selected using NS - the only mention I found was in the DESCRIPTION, saying that some substitution and clock models are available. It would be useful to include this in the README along with a list of available models. One example showing clock model comparison, in the vignette for instance, would also be useful.

I agree info on this was thin. I've created a vignette in beautier that showcases the inference model options.
In README, vignette and paper, I added the available site, clock and tree models.

in general, the documentation assumes that people using mcbette will already be familiar with the other packages of the suite, particularly beautier. I think this expectation needs to be stated much more clearly in the README and vignette, and that a link to an appropriate introduction vignette should be provided.

I've described the relation to the beautier package in more places and link to a vignette in beautier that showcases the inference model options.

the example in the README should describe the tested model briefly (JC69, strict clock, etc).

I added this.

there is a create_ns_inference_model function in the package, but the examples (README, vignette, package doc) use the beautier::create_test_ns_inference_model function instead - it's unclear what the difference is between the two, or which one should be used for a real analysis. If the test function is only used to set up a shorter analysis (as the README seems to imply ?), then it would be better to set the shorter analysis with the real function and modified parameters, and explain what is being done.

I moved create_ns_inference_model to the beautier package and made the documentation of the create_[something]_inference_model functions link to each other.

Additionally, I followed your suggestion and replaced:

x <- create_ns_test_inference_model()

by

x <- create_ns_inference_model()
x$mcmc <- create_test_ns_mcmc()

in the default parameter documentation, beast2_options should link to the create_mcbette_beast2_options function used in all the examples, instead of the beastier::create_beast2_options function. Similarly inference_model should link to either create_ns_inference_model or beautier::create_test_ns_inference_model rather than beautier::create_inference_model.

I made these link to one another. Note that the inference_model functions have been moves to beautier, and the beast2_options functions to beastier

when autocompleting in RStudio, the description shown on the function is the beginning of the Description section. This means that for instance check_mcbette_state shows only Will stop otherwise.. Same issue with check_beast2_ns_pkg, interpret_bayes_factor.

This is a neat observation, that I was unaware of. I have fixed this.

contrary to what the doc says, est_marg_lik does not return either estimates or trees.

Well spotted! I've updated the doc in this commit.

the error message when est_marg_lik fails prints the inference model nicely, but not the BEAST2 options (it shows beast2_1c6e311fc10d3.xml.stateNANAFALSETRUE).

Hmmm, I wish I could reproduce that! Because if I look into the code, I do not treat the inference model different than the BEAST2 options:

  tryCatch({
      bbt_run_out <- babette::bbt_run_from_model(
        fasta_filename = fasta_filename,
        inference_model = inference_model,
        beast2_options = beast2_options
      )
      ns <- bbt_run_out$ns
    },
    error = function(e) {
      stop(
        "Could not estimate the marginal likelihood. \n",
        "Error message: ", e$message, "\n",
        "fasta_filename: ", fasta_filename, "\n",
        "beast2_options: ", beast2_options, "\n",
        "inference_model: ", inference_model, "\n"
      )
    }

I would enjoy fixing this, but if I cannot check my results, I cannot see if I have fixed it. Could you reproduce the bug? If yes, I will happily add it!

it would be useful to have an example in the interpret_bayes_factor doc of how to use that function with the output from est_marg_liks.

I agreed, up until I added interpret_marg_lik_estimates, which interprets the results of est_marg_liks (ropensci/mcbette#34), which -IMHO- makes the interpret_bayes_factor redundant. I hope you agree!

Minor comments

the doc for est_marg_lik links to itself, same with est_marg_liks

Well spotted! It now links correctly to est_marg_liks

* `est_marg_lik.R`, l16 "Actual" -> "Current"

Renamed.

Functionality

the main feature I think is missing from the package at the moment is the ability to keep the log and tree output of the babette runs, in addition to the marginal likelihood. The documentation says that the trees are returned, but that does not appear to be the case. I would expect that users doing model selection will also be interested in the parameter estimates for the best model.

To the mcbette main documentation (?mcbette) and mcbette::est_marg_liks I've added how to obtain the parameter estimates, posterior trees and other things stored in temporary files.

my other suggestion is to have a function similar to interpret_bayes_factor which could directly process the output from est_marg_liks, and print the table with the best model highlighted and how all the others compare to that one.

Great idea, I did so by adding interpret_bayes_factor_log_lik (ropensci/mcbette#33). It even comes with a text plot!

JOSS paper comments

* two references are missing DOIs (required according to JOSS guidelines)

I added DOIs to all journal articles.

* there is a `> Example text below` left in l18

Blimey, well spotted! I've put the Summary there 👍

* the single quotes in the title (`'babette'`) give me a yaml parser error when building the file

I removed these quotes altogether. I've added a Travis CI test to verify that knitr::knit does work on the file.

* l59-61 I think you need to clarify that you are only talking about living species here rather than all species. It's also unclear to me why you say we "should be able to reconstruct" rather than "can reconstruct".

Agreed, I've used to wording you suggested.

* l61-62 I would introduce phylogenies earlier, when showing the primates tree.

I rewrote most of the paper, after which phylogenies are introduced sooner.

* l70-71 I think this sentence is too vague. I would suggest replacing it with "However, constructing a phylogeny is non-trivial, as it requires the researcher to choose how to model the evolutionary process, including..."

Agreed, I've used your more precise wording. Thanks!

* l80-85 This section needs to mention that the 'best' model is always defined in regards to a particular dataset. It's explained later but needs to be made clear earlier.

I have reworded in a such a way to avoid using the word 'best', and it includes that it is dependent on the alignment/dataset:

mcbette is an R package that determines the model with the most
evidence (aka marginal likelihood) for having generated the alignment,
from a set of models.

* l85 I don't think you can reuse this definition of 'best' for phylogenies - what would it mean for a phylogeny to be "simple" ?

Agreed, I removed it. Thanks!

Minor comments

* l49 then -> than

Fixed. Thanks!

* l63 a the -> the

Fixed. Thanks!

• Add bjoelle to DESCRIPTION as reviewer

Did so.

[richelbilderbeek]

Second reviewer's reply:

Documentation

The package includes all the following forms of documentation:

[ ] A statement of need clearly stating problems the software is designed to solve and its target audience in README
A statement is present, but I do not think it states the problem clearly enough. What is a "Model Comparison"? The phrase is too generic to give a user a sense of what mcbette intends to solve. Although someone familiar with BEAST can likely guess this for him/herself, it should not be up to the reader to interpret. I don't think the author needs to go into a whole lot of detail, but I'd suggest adding 2-3 more sentences to elaborate on what the point of fitting the models is and why the models are being compared at the top of the readme where the package is introduced. There's a few sentences in the paper.md that achieve this nicely, so I suggest just incorporating them here as well. I'd also rephrase "solve a common problem" in the Example section to give an explanation of what the problem in the example is and why it is being solved. Again, I anticipate that a user of this package will likely know how all this works. Rather, the point here is to make it more immediately clear what mcbette does when a user first encounters the package.

Indeed, it was a bit concise. I have rephrased this as such:

mcbette allows for doing a Model Comparison using babette (hence the name),
that is, given a (say, DNA) alignment, it compares multiple phylogenetic inference
models to find the model that is likeliest to generate that alignment.
With this, one can find the phylogenetic inference model that is
simple enough, but not too simple.

To do so, mcbette uses babette [Bilderbeek and Etienne, 2018] with the addition
of using the BEAST2 [Bouckaert et al., 2019] nested sampling package
as described in [Russell et al., 2019].

[x] Installation instructions: for the development version of package and any non-standard dependencies in README
In contrast to the other reviewer, I did not encounter any issues with installation or running code (macOS 10.15.4).

Great!

[x] Vignette(s) demonstrating major functionality that runs successfully locally
Great work! Nice explanation of a clear example. I'll suggest linking this on the readme too as it is very helpful for understanding what the package does and how it works.

Great!

[x] Function Documentation: for all exported functions in R help

Happy to read I did a good job 👍

[x] Examples for all exported functions in R Help that run successfully locally

Happy to read I did a good job 👍

[x] Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Happy to read I did a good job 👍

For packages co-submitting to JOSS

• The package has an obvious research application according to JOSS's definition

The package contains a paper.md matching JOSS's requirements with:

[ ] A short summary describing the high-level functionality of the software

Indeed, somehow this short summary is missing! I've added it 👍

Some of this may be a matter of style, but I have a bit of feedback about how this paper is written. Firstly, the introductory material (all paragraphs and figures before "Constructing a phylogeny, however, is non-trivial...") are too basic for my taste -- I don't think evolution itself needs to be described in fundamental detail at all. This explanation is not appropriate for the package's audience -- we know all this already. Rather, I suggest starting with the logic of paragraph 4 ("Constructing a phylogeny, however, is non-trivial...") which states the problem at hand nicely. Although I like the core idea of this paragraph, the author may also consider fleshing out an explanation of why statistical models are used at all.

Thanks to your feedback, I have removed the whole -indeed slow- introduction and started at the point you suggested. With some rewriting, I feel I have introduced the need for statistical models enough (by introducing the term 'evidence', aka marginal likelihood) and I hope you agree.

What I will also more strongly recommend is that the author give the reader a sense of where to start when using the package. Broadly speaking, what is the analytical pipeline one should use with mcbette? I think inclusion of a worked example in the paper would help users know where to start, how to structure their data, which functions to look into, and which vignettes they may want to look into. This would be great to have as either the penultimate or the final paragraph. I don't think it needs to go into specific detail, but just give the reader a general sense of how the package works.

Agreed! I have added a section 'Getting Started' that should introduce the reader to the field and direct him/her to the literature.

• Authors: A list of authors with their affiliations
• A statement of need clearly stating problems the software is designed to solve and its target audience.
• References: with DOIs for all those that have one (e.g. papers, datasets, software).
  As noted by the other reviewer, some DOIs are missing and it would be great to have them included

Agreed, thanks! I have added these!

Functionality

[x] Installation: Installation succeeds as documented.
Nice touch with the can_run_mcbette() function! Very handy!

Thanks! If you like that one, you'll love mcbette_report 😎 👍

• Functionality: Any functional claims of the software been confirmed.

Cool!

• Performance: Any performance claims of the software been confirmed.

Awesome.

• Automated tests: Unit tests cover essential functions of the package
  and a reasonable range of inputs and conditions. All tests pass on the local machine.

I am unsure why, but testing failed on my machine. Here are the relevant lines
from the devtools check:

── Test failures ───────────────────────────────────────────────── testthat ────

library(testthat)
library(mcbette)
before <- get_mcbette_state()
test_check("mcbette")
── 1. Error: Package style (@test-style.R#2) ───────────────────────────
invalid 'path' argument
Backtrace:

1. lintr::expect_lint_free()
2. lintr::lint_package(...)
3. base::normalizePath(path, mustWork = FALSE)
4. base::path.expand(path)

══ testthat results ════════════════════════════════════════════════════
[ OK: 88 | SKIPPED: 5 | WARNINGS: 0 | FAILED: 1 ]

1. Error: Package style (@test-style.R#2)

Error: testthat unit tests failed
Execution halted
1 error x | 0 warnings ✓ | 1 note x
Error: R CMD check found ERRORs
Execution halted
Exited with status 1.

I have the same error. This was not the case upon the submission of mcbette.
A bug report has already been filed here in October 2019.
I've removed the tests, as it is tested by Travis CI anyways.

Final approval (post-review)

• The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Let's see what this reviewer thinks about this 👍

Estimated hours spent reviewing: ~3.5

* [x]  Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Done, including the ORCID ID 👍

Review Comments

Firstly, I am quite happy to see that this package exists and was submitted to rOpenSci. mcbette is poised to be very helpful towards biologists who work on tree inference and phylogenetic comparative analyses. By using BEAST and accompanying software, mcbette builds off the author's previous babette and helps expand a user's ability to rely on reproducible pipelines for analyzing genetic data. As someone who will likely use babette and other packages in this ecosystem in the near future, I appreciate what mcbette brings to the table: giving a reproducible approach to model comparisons of sequence evolution.

In running the worked examples as well as feeding in sequences from my own data sets, I did not encounter any issues with how the code ran or the results it produced. To the best of my ability to assess this, mcbette seems to produce correct results.

Happy to hear this!

That being said, testing failed on my machine. Please see above for the relevant lines from the devtools check output. Also in investigating testthat.R, the result of line 6 test_check("mcbette") was:

Error: No tests found for mcbette

As noted above, the devtools issues are caused by something upstream (in the lintr package) and needs to be resolved there.

Regarding the second error, I can perfectly reproduce this. I predict this is a feature, not a bug. With CTRL+SHIFT+T in R Studio or devtools::test() the tests do start as expected. Also, mcbette is tested by Travis CI (for Linux and Mac) and AppVeyor (for Windows) and its code coverage is measured from these tests. Therefore, I assume the tests works and that the No tests found for mcbette message is some R feature I and you misinterpret.

Unlike the other reviewer, I did not encounter issues during install (using macOS 10.15.4 with R 3.6.2 and then again on R 4.0.0) so I am sorry to not be able to offer any insight on troubleshooting. I should note that I already had rJava installed, which may help explain the lack of problems.

I am happy to have this confirmed 👍

My inline comments above are geared towards enhancing the clarity of the documentation. I will agree with the other reviewer's assessment -- the documentation and the accompanying paper are a little too sparse as presently written. I'd appreciate seeing expansions of both the high-level functionality (in a broad sense) as well as more detailed descriptions of how the author envisions users feeding data into analytical pipelines. I don't mind having similar information repeated in the readme, description, vignettes, and paper; doing so helps not only reinforce the message of the package and also helps users' understanding regardless of their point of entry into the package.

I added quite some documentation, due to feedback from the other reviewer indeed. I hope this will also satisfy your needs.

I agree with all the specific bullet points the other reviewer raised regarding the Documentation and Functionality sections and for the sake of brevity won't repeat them here. I'll add that providing a full(er) list of potential models would be great to have, perhaps as its own vignette.

I do not necessarily agree with the other reviewer's comments regarding the JOSS paper but this is primarily because I feel the paper should be restructured (please see inline comments above).

I have restructured the paper from a mix of both reviews. I hope you both like it. I, for sure, do!

Thanks again for the opportunity to review this package. Again, my suggestions are motivated by wanting to enhance the clarity of presentation and make it easier for potential users to see the value of relying on mcbette. It's nice work!

turtle

Thanks so much for reviewing!

[richelbilderbeek]

@richelbilderbeek thank you! could you please post your response in this thread?

@maelle: Here goes! A bit clumsy that I forgot 😊 ....

[maelle]

Thank you, no problem!

@vbaliga @bjoelle are you satisfied with @richelbilderbeek's response? If so would you mind checking the box " The author has responded to my review and made changes to my satisfaction. I recommend approving this package." in your review and if not would you be able to add some comments in this thread? In any case many thanks already and again for your reviews.

[vbaliga]

Rather than edit my previous review, I am electing to write a fresh one here:

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• Briefly describe any working relationship you have (had) with the package authors.
• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README

• Installation instructions: for the development version of package and any non-standard dependencies in README

  Again, I had no trouble with installation but note that I already use rjava for other things

• Vignette(s) demonstrating major functionality that runs successfully locally

• Function Documentation: for all exported functions in R help

• Examples for all exported functions in R Help that run successfully locally

• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

For packages co-submitting to JOSS

• The package has an obvious research application according to JOSS's definition

The package contains a paper.md matching JOSS's requirements with:

• A short summary describing the high-level functionality of the software

• Authors: A list of authors with their affiliations

• A statement of need clearly stating problems the software is designed to solve and its target audience.

Very nicely done!

• References: with DOIs for all those that have one (e.g. papers, datasets, software).

As noted by the other reviewer, some DOIs are missing and it would be great to have them included

Functionality

• Installation: Installation succeeds as documented.

• Functionality: Any functional claims of the software been confirmed.

• Performance: Any performance claims of the software been confirmed.

• Automated tests: Unit tests cover essential functions of the package
  and a reasonable range of inputs and conditions. All tests pass on the local machine.

• Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

• The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: <1 for this round

• Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

---

Review Comments

Thanks very much for taking my feedback into account. This package was already very good and I think it is even better now in the sense that it is more approachable for new users.

I have a couple minor points to bring up, but please note that despite these points I am satisfied with the changes I already see and I am supportive of this work's publication.

During devtools::check(), I encountered:

> checking R code for possible problems ... NOTE
  interpret_marg_lik_estimates: no visible global function definition for
    ‘na.omit’
  Undefined global functions or variables:
    na.omit
  Consider adding
    importFrom("stats", "na.omit")
  to your NAMESPACE file.

The other point is that I think it would be a nice touch to link to the vignettes at the end of paper.md. The paper does a nice job giving an overview of the package and it would be great to end it by giving users a suggestion of what they might want to read next, i.e. the vignettes.

Thanks again to @maelle for the opportunity to review this and to @richelbilderbeek for taking my feedback. I am looking forward to using mcbette and Richel's other packages in the near future.

Best regards,
Vikram

🐢

[richelbilderbeek]

@vbaliga is right: I will process his feedback in this weekend I already processed his feedback 👍

[bjoelle]

Sorry it took me so long to come back to this review ! Overall I am happy with the changes that have been made to the package and documentation.
My one comment would be that I feel like the current version of the JOSS paper underplays the usefulness of the package (the statement of need in particular feels a bit weak). I would state more clearly what the added value of mcbette is (compared to using BEAST2 directly) : easy set-up and execution of several parallel analyses using different models, easy analysis and nice printing of the results of the model selection, possibility to integrate it into an existing R pipeline, etc.

[maelle]

Thank you @bjoelle! The JOSS comments are very important. @richelbilderbeek note that JOSS recently changed their scope so please read the scope again before submitting https://joss.theoj.org/about#submitting

Approved! Thanks @richelbilderbeek for submitting and @vbaliga @bjoelle for your reviews! 😸
To-dos:

• Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
• Fix all links to the GitHub repo to point to the repo under the ropensci organization.
• Delete your current code of conduct file if you had one since rOpenSci's default one will apply, see https://devguide.ropensci.org/collaboration.html#coc-file
• If you already had a pkgdown website and are ok relying only on rOpenSci central docs building and branding,
  • deactivate the automatic deployment you might have set up
  • remove styling tweaks from your pkgdown config but keep that config file
  • replace the whole current pkgdown website with a redirecting page
  • replace your package docs URL with https://docs.ropensci.org/package_name
  • In addition, in your DESCRIPTION file, include the docs link in the URL field alongside the link to the GitHub repository, e.g.: URL: https://docs.ropensci.org/foobar (website) https://github.com/ropensci/foobar
• Fix any links in badges for CI and coverage to point to the ropensci URL. We no longer transfer Appveyor projects to ropensci Appveyor account so after transfer of your repo to rOpenSci's "ropensci" GitHub organization the badge should be [![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/github/ropensci/pkgname?branch=master&svg=true)](https://ci.appveyor.com/project/individualaccount/pkgname). If Appveyor does not pick up new commits after transfer, you might need to delete and re-create the Appveyor project. (Repo transfers are smoother with Travis CI and GitHub Actions)
• We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.
• Activate Zenodo watching the repo
• Tag and create a release so as to create a Zenodo version and DOI
• Submit to JOSS at https://joss.theoj.org/papers/new, using the rOpenSci GitHub repo URL. When a JOSS "PRE REVIEW" issue is generated for your paper, add the comment: This package has been reviewed by rOpenSci: https://LINK.TO/THE/REVIEW/ISSUE, @ropensci/editors

Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent). More info on this.

Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @stefaniebutland in your reply. She will get in touch about timing and can answer any questions.

We've put together an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here.

[richelbilderbeek]

Great! Will do so ASAP, probably next weekend 👍

[maelle]

Ok, thanks for the update! 🙏

[richelbilderbeek]

Thank you @bjoelle! The JOSS comments are very important. @richelbilderbeek note that JOSS recently changed their scope so please read the scope again before submitting https://joss.theoj.org/about#submitting

I re-read the scope. AFAICS, the article is still within the scope.

To-dos:

* [x]  Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo.  I have invited you to a team that should allow you to do so.  You'll be made admin once you do.

Done 👍

* [x]  Fix all links to the GitHub repo to point to the repo under the ropensci organization.

I did a 'replace all' using a regex searching for all files with richelbilderbeek/mcbette and replaced it with ropensci/mcbette

* [x]  Delete your current code of conduct file if you had one since rOpenSci's default one will apply, see https://devguide.ropensci.org/collaboration.html#coc-file

Done.

* [x]  If you already had a `pkgdown` website **and are ok relying only on [rOpenSci central docs building and branding](https://devguide.ropensci.org/ci.html#even-more-ci-ropensci-docs)**, [...]

Not applicable

* [x]  Fix any links in badges for CI and coverage to point to the ropensci URL. We no longer transfer Appveyor projects to ropensci Appveyor account so after transfer of your repo to rOpenSci's "ropensci" GitHub organization the badge should be `[![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/github/ropensci/pkgname?branch=master&svg=true)](https://ci.appveyor.com/project/individualaccount/pkgname)`. If Appveyor does not pick up new commits after transfer, you might need to delete and re-create the Appveyor project. (Repo transfers are smoother with Travis CI and GitHub Actions)

Done 👍

* [x]  We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.

Done 👍

* [ ]  Activate Zenodo watching the repo

Unsure if I did so correctly. I did create a DOI for the current version, 10.5281/zenodo.4076183

* [x]  Tag and create a release so as to create a Zenodo version and DOI

Done, DOI is 10.5281/zenodo.4076183

* [x]  Submit to JOSS at https://joss.theoj.org/papers/new, using the rOpenSci GitHub repo URL. When a JOSS "PRE REVIEW" issue is generated for your paper, add the comment: `This package has been reviewed by rOpenSci: https://LINK.TO/THE/REVIEW/ISSUE, @ropensci/editors`

Done so, at openjournals/joss-reviews#2738

Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field (with their consent). More info on this.

Done 👍

Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @stefaniebutland in your reply. She will get in touch about timing and can answer any questions.

Hi @stefaniebutland, I would enjoy to write a blog post about mcbette. I would enjoy it to work together again 👍

We've put together an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here.

I did not need the book this time...

[stefaniebutland]

Congratulations @richelbilderbeek on another package passing peer review! Do you think this one would be suitable as a tech note, rather than longer narrative blog post?

Our Blog Guide has details to help you decide, and provides technical details for submitting your post. I think we published this after your babette post.

Please suggest a date by which you would like to submit a draft via pull request and I will reply with a publication date. My colleague Steffi LaZerte will review.

[maelle]

Thanks @richelbilderbeek !

[richelbilderbeek]

Hi @stefaniebutland,

Do you think this one would be suitable as a tech note, rather than longer narrative blog post?

I would prefer the longer narrative blog post, as this one is for a broader audience. I do wonder if that is not too similar to the JOSS acticle I submitted. What do you think?

• If you think the JOSS article is too similar to a longer narrative blog post, I will happily write a tech note
• Else, I will happily write a longer narrative blog post

👍

[stefaniebutland]

I'm happy to leave this up to your judgement @richelbilderbeek. The post will reach a different audience, so a good way to get more attention for it. That will work as long as you make the content different enough from the JOSS paper. A blog post allows for a somewhat more casual tone.

You may use 2020-11-17 or 2020-12-01 as a tentative publication date; date to be finalized during review.

[stefaniebutland]

Hi @richelbilderbeek. Do you prefer 2020-11-17 or 2020-12-01 as a tentative publication date? If 2020-11-17, fyi my colleague @steffilazerte will review on 2020-11-13, so a short turnaround time for edits.

[richelbilderbeek]

@stefaniebutland I prefer 2020-12-01, as I will not make it sooner. I will submit a version fit for review before Sunday 2020-11-22 23:59, is that soon enough?

[stefaniebutland]

Perfect. Thank you