Joss-reviews: [REVIEW]: Phylen: automatic phylogenetic reconstruction using the EggNOG database

Created on 22 Feb 2018  ยท  62Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @giraola (Gregorio Iraola)
Repository: https://github.com/iferres/phylen
Version: v0.1.0
Editor: @Kevin-Mattheus-Moerman
Reviewer: @juanvillada
Archive: 10.6084/m9.figshare.6223538.v1

Status

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/aeedb5e6a0e6d0cd88ec273eb50e1254"><img src="http://joss.theoj.org/papers/aeedb5e6a0e6d0cd88ec273eb50e1254/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/aeedb5e6a0e6d0cd88ec273eb50e1254/status.svg)](http://joss.theoj.org/papers/aeedb5e6a0e6d0cd88ec273eb50e1254)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

## Reviewer instructions & questions

REVIEWER 1

@juanvillada, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

### Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: Does the release version given match the GitHub release (v0.1.0)?
  • [x] Authorship: Has the submitting author (@giraola) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

REVIEWER 2

@kortschak, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

### Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: Does the release version given match the GitHub release (v0.1.0)?
  • [x] Authorship: Has the submitting author (@giraola) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

REVIEWER 3

@KlausVigo, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

### Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: Does the release version given match the GitHub release (v0.1.0)?
  • [x] Authorship: Has the submitting author (@giraola) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
accepted published recommend-accept review

All 62 comments

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @juanvillada it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews ๐Ÿ˜ฟ

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands
Attempting PDF compilation. Reticulating splines etc...

@giraola @iferres can you comment on testing your code or point @kortschak to a "test suite"?

Thanks @Kevin-Mattheus-Moerman. @kortschak Thanks for your comment. I'm preparing a real case example to improve the README.md, I will add some data to the repo so it can be tested.

Hi @kortschak, @Kevin-Mattheus-Moerman and @juanvillada, thanks for accepting the revision of our manuscript. Following your recommendations we want to include a full test situation, however we think it has no sense to upload the full genomes into the repo (genomes are also public), so do you think pointing to a download link for the test data would be ok?

@kortschak Please check the repo new README.md file. I have also attached example data, so you may have to re install the package.

@KlausVigo I've just added a tickbox set for you as well

@whedon assign @kortschak as reviewer

OK, the reviewer is @kortschak

@whedon assign @KlausVigo as reviewer

OK, the reviewer is @KlausVigo

@whedon assign @juanvillada as reviewer

OK, the reviewer is @juanvillada

@juanvillada (reviewer 1) @kortschak (reviewer 2) @KlausVigo (reviewer 3) we are good to go for the review process. Let me know if you are not able to tick the boxes at the top of this issue (be sure to accept this invite: https://github.com/openjournals/joss-reviews/invitations).
If you have questions just ping me or check out the reviewer guidelines. THANKS! :tada:

@giraola Thank you for making a start on testing. I think that in the context of this tool integration testing is going to be very useful from a number of perspectives: 1) making people trust that the software behaves as advertised, 2) guards against regressions and 3) provides an idiomatic recipe for users to work from, so yes, I think that it should be OK to run a test on downloaded data (it might be worth letting the package do the download and cache it for convenience and efficiency), but I think that a smaller test set that is packaged with the code is also important. Extending this, while I can see that much of the code here is glue and doesn't lend itself well to unit testing, there are functions where that is easy to do, and adding unit tests will help you maintain to code in the future, so I'd suggest adding unit tests for most of the functions in gff3_Handling.R and some of the functions in utils.R.

Hadley Wickham has a nice package for testing that many people recommend: https://github.com/r-lib/testthat.

@kortschak thank you for these suggestions. We are exploring testthat to add unit tests to the main functions of phylen. In parallel, we already added a fully packaged example with step-by-step explanation to run it.

@giraola For the general checks, the question is "Has the submitting author (@giraola) made major contributions to the software?" Interpreting this literally it is hard to say yes based on git blame/git log, but I imagine that your input has been supervisional and I should be reading the author list as a biologist. Is this correct?

@Kevin-Mattheus-Moerman Can you clarify the interpretation of this question?

Hi @kortschak thanks for your question. In this case there are two contributors both of which are authors and appear to agree on their joint authorship. The check mark you mention really serves to weed out submission where somebody has for instance forked somebody else's work and submitted as main author without having made any significant contributions. Other contributors might object to such a person claiming main authorship. Anyway, this is clearly not the case here.

Thanks @Kevin-Mattheus-Moerman. Maybe the question could be reworded to something like "Have the authors made major contributions to the software?"

Hi @kortschak thanks for your question. Indeed, @iferres mainly contributed to develop the code within the repository, but both have made major contributions to the software. I am @iferres's Master thesis supervisor and my contributions were mainly conceptual.

@giraola I will be able to look at the functionality criterion when the tests are added, so after that and the addition of the correct version tag, I'll be able to complete my review.

@kortschak we are expecting to finish the tests in the following days, we will notify you when done. Thanks.

@kortschak @juanvillada tests are already added, you can have a look now in the repo. @kortschak which is wrong with the version tag?

Thanks @giraola, already did a preliminary "manual" test. Will do the proper test asap and post a couple of comments later this week.

@juanvillada There was a typo on one of the test scripts which was making the test to fail, now everything is working.

@giraola There is no version tagged in the repository.

I am unable to run the tests successfully:

> library(testthat)
> library(phylen)
Loading required package: phangorn
Loading required package: ape
> test_package(phylen)
Error in match(x, table, nomatch = 0L) : 
  'match' requires vector arguments
> sessionInfo()
R version 3.4.4 (2018-03-15)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 16.04.4 LTS

Matrix products: default
BLAS: /usr/lib/atlas-base/atlas/libblas.so.3.0
LAPACK: /usr/lib/atlas-base/atlas/liblapack.so.3.0

locale:
 [1] LC_CTYPE=en_AU.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_AU.UTF-8        LC_COLLATE=en_AU.UTF-8    
 [5] LC_MONETARY=en_AU.UTF-8    LC_MESSAGES=en_AU.UTF-8   
 [7] LC_PAPER=en_AU.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_AU.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] phylen_0.1.0   phangorn_2.4.0 ape_5.0        testthat_2.0.0

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.16    quadprog_1.5-5  lattice_0.20-35 XML_3.98-1.10  
 [5] MASS_7.3-40     grid_3.4.4      R6_2.2.2        nlme_3.1-131.1 
 [9] magrittr_1.5    httr_1.3.1      rlang_0.2.0     seqinr_3.4-5   
[13] Matrix_1.2-11   fastmatch_1.1-0 ade4_1.7-10     igraph_1.2.1   
[17] parallel_3.4.4  compiler_3.4.4  pkgconfig_2.0.1

It might be worthwhile setting up a CI service job with TravisCI or similar so that tests are run on commit.

I think "phylen" should be between quotes in test_package function. Anyway, I still got an error:
``

test_package("phylen")
Error: No tests found for phylen

``
which is strange because it runs ok when I use Ctrl + Shift + T inside RStudio. Let me check what's going on and I let you know.
I will check the CI stuff.
Thanks for all the recommendations, by the way, its my first package on revision and I'm learning a lot with your suggestions.

Ok, added TravisCI and the status is "passing". @kortschak @juanvillada

Regarding version: Is written at the DESCRIPTION file. Do you think it should be more visible?

@iferres Thank you for doing that. Testthat is not wildly helpful in this kind of context I see.

For the version, at some point you will need to make a release, github will require that this is a tagged commit and the JOSS requirement is that the release matches version here "Does the release version given match the GitHub release (v0.1.0)?" This is obviously impossible if followin good git tag hygiene and changes are requested, but after everyone is happy with the state of the code/text, then you should tage the HEAD with v0.1.0 and make that the release for your doi store submission.

Thank you for bearing with me here.

@kortschak Ok, done. Thanks for your patience explaining this. Please, confirm if what I did is what is needed.
Regarding the test_package issue, I found this.

That works (hopefully there won't need to be a bump, but it looks like there won't be - in my submissions it has gone in as v1.0.0 and become v1.0.1 by the time review has gone through).

Thanks for the link (I filed an issue at testthat asking for improvement of the docs).

Hi @Kevin-Mattheus-Moerman, we have addressed the helpful suggestions made by @kortschak to improve Phylen. We would like to know if @juanvillada or @KlausVigo have further questions/comments.

Hi @giraola and @iferres,

I just made a pull request, to make the default closer to what users would expect. The optim.pml should also some tree rearrangements, otherwise it would just return an optimised NJ tree. Additionally I added some code which sets the number of rate categories to 4 if optimisation of the discrete gamma distribution is requested. That is probably some of the more quirky things of using pml and optim.pml in phangorn. You want to test that it works properly.

I like that you added travis-ci support. It is actually useful to try also eliminate all notes and warnings messages from R CMD check. The other small commit was just doing that. You may have a copy of the paper.md also on the vignettes folder and the file readme_img1.png in that folder seems to be just a relict of an older document.

This is not really the objective of this review, but I think it should be possible to distribute the package to other operation systems not just linux with only minor changes. You would only need an option to set the path to the binaries or ensure that the binaries are on the search path from hmmr, mafft. It is done in ape for the example in the muscle function. It still is a nightmare if you give a course and half the students don't know where their binaries are sitting on the disc. This would allow to upload the package on CRAN and to reach a wider audience of users.

Regards,
Klaus

Hi @KlausVigo thanks for your time and for optimising the tree building step. We will have a look at muscle() function in APE. We agree that would be better to distribute the package to other OSs, indeed some colleagues have already requested this!

Hi @giraola and @iferres,

Summary of my review:

  • โœ… General Checks
  • โš ๏ธ Functionality
  • โœ… Documentation

    - โœ… Software paper

โœ… General Checks

I had two comments for this section on Version and Authorship. They were addressed already.

โš ๏ธ Functionality

Installation

  • Assuming some people do not have it previously installed, I would suggest to include a line to install the devtools package.
install.packages("devtools")
devtools::install_github("iferres/phylen")

Minor: I don't think you must include this in the installation docs, but only in case you find this useful, I got phylen to work on a new and clean Ubuntu distro only after doing this:

install.packages("git2r")
install.packages("httr")
install.packages("devtools")
install.packages("XML")
devtools::install_github("iferres/phylen")
library(phylen)
  • In order to avoid people getting stuck with requirements, I would suggest to include some lines in order to faciliate MAFFT and HMMER installation:

Assuming homebrew (macOS) or linuxbrew (Linux) availability:

brew install brewsci/science/mafft
brew install brewsci/science/hmmer

or with apt:

sudo apt install mafft
sudo apt install hmmer
  • I tested phylen with testthat. It is OK.
library(phylen)
install.packages("testthat")
library(testthat)
test_package("phylen")
> test_package("phylen")
โ•โ• testthat results  โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•
OK: 23 SKIPPED: 0 FAILED: 0
  • โ—๏ธManual test. I have not been able to contruct the final tree following the instructions in README.md, see the output below:

RStudio console output

> p <- phylen(gffs = gff, # The gff files extracted on the first step
+             hmmFile = hmm, # The downloaded HMMs
+             isCompressed = TRUE, # The downloaded HMMs are compressed (tar.gz)
+             phyloMode = "ml", # nj or ml, in this case we will use maximum likelihood
+             nbs = 100, # The number of bootstrap.
+             level = 100, # The percentage of genomes a gene must be in to be considered as part of the coregenome.
+             outDir = "phylen", # Lets create a directory called "phylen" to put the output files
+             n_threads = 2) # Use 1 threads
Decompressing.. DONE!
Concatenating.. 
===============================================================================================================
DONE!
Pressing.. DONE!
Getting information from hmms.. DONE!
Searching.. DONE!
Computing panmatrix.. DONE!
Getting core-genes.. DONE!
Writting fastas.. DONE!
Aligning.. sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not foundsh: 1: 
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: sh: 1: linsi: not found
linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
sh: 1: linsi: not found
DONE!
Concatenating.. DONE!
Removing intermediate files.. DONE!
Generating phylogeny..
Error in data[[1]] : subscript out of bounds
In addition: There were 22 warnings (use warnings() to see them)
> p
Error: object 'p' not found

โœ… Documentation

  • It would be nice to add some topics "_to categorize your repository and make it more discoverable_โ€.

โœ… Software paper

Seems fine now after DOIs correction ๐Ÿ‘

Thanks @KlausVigo. Some of the suggestions were already addressed. I think CRAN doesn't allow to add binaries directly on the repo anymore, but the source files can be added and compilation is done when installing the package. Since HMMER and MAFFT are 2 widely used tools in bioinformatics (and easy to install), I thought that asking the user to have them already installed would be simpler, but it is worth considering adding source files. In concordance with the above, I see that @juanvillada had some problems running phylen and it seems to be that linsi was not installed together with MAFFT (which is just an alias for mafft --maxiterate 1000 --localpair). I think the best way to solve this problem is to add mafft source files to the package and compile them when installing phylen, but because this could take me some time, meanwhile I added some information at README and to the manual pages about the need to install these shortcuts together with mafft (they are installed when downloading MAFFT from its webpage, but I think not when installing MAFFT using a package manager (apt, brew, etc.) ). Now, when run phylen() function, it checks first for these aliases to be installed on $PATH and exit if not with a warning.
Added some topics.

Hi @iferres ,

I think these changes would be enough for the start. I didn't want to suggest that you need to add the sources of HMMER and MAFFT and install them with the package, that is probably too much work. If you could be done it is super useful of course.

There is also the msa package on Bioconductor, which installs muscle and clustal to use from inside R. If the maintainer would add MAFFT, this would be really great and one would have all alignment programs in one package. The maintainer Ulrich Bodenhofer seems a really nice guy (and he got Ripleyed because of me), so may try to get in contact with him. And there are some function in phangorn / ape to convert between the different R alignment formats.

Have a nice weekend

Cheers,
Klaus

Hi @Kevin-Mattheus-Moerman, we would like to know what additional specific modifications are needed to fulfil reviewer's requirements. These are boxes that remain unchecked:

  • @juanvillada > Functionality
  • @kortschak > Functionality
  • @KlausVigo > Version, Statement of need, Functionality documentation.

Cheers,
Gregorio

Hi @giraola and @iferres!

I reinstalled and tried phylen again today. It is working as expected.
Functionality OK!

> devtools::install_github("iferres/phylen")
> library(phylen)
> tgz <- system.file('extdata', 'toydata.tar.gz', package = 'phylen')
> gff <- untar(tarfile = tgz, exdir = getwd(), list = T)
> untar(tarfile = tgz,exdir = getwd())
> hmm <- download_nog_hmm(nog.prefix = "eproNOG", onDir = getwd())
> p <- phylen(gffs = gff, # The gff files extracted on the first step
            hmmFile = hmm, # The downloaded HMMs
            isCompressed = TRUE, # The downloaded HMMs are compressed (tar.gz)
            phyloMode = "ml", # nj or ml, in this case we will use maximum likelihood
            nbs = 100, # The number of bootstrap.
            level = 100, # The percentage of genomes a gene must be in to be considered as part of the coregenome.
            outDir = "phylen", # Lets create a directory called "phylen" to put the output files
            n_threads = 2) # Use 2 threads



Decompressing.. DONE!
Concatenating.. 
==============================================================================================================
DONE!
Pressing.. DONE!
Getting information from hmms.. DONE!
Searching.. DONE!
Computing panmatrix.. DONE!
Getting core-genes.. DONE!
Writting fastas.. DONE!
Aligning.. DONE!
Concatenating.. DONE!
Removing intermediate files.. DONE!
Generating phylogeny..
optimize edge weights:  -5958264 --> -5731301 
optimize base frequencies:  -5731301 --> -5685751 
optimize rate matrix:  -5685751 --> -5620789 
optimize edge weights:  -5620789 --> -5619310 
optimize topology:  -5619310 --> -5619310 
0 
optimize base frequencies:  -5619310 --> -5615981 
optimize rate matrix:  -5615981 --> -5614940 
optimize edge weights:  -5614940 --> -5614927 
optimize base frequencies:  -5614927 --> -5614698 
optimize rate matrix:  -5614698 --> -5614643 
optimize edge weights:  -5614643 --> -5614640 
optimize base frequencies:  -5614640 --> -5614626 
optimize rate matrix:  -5614626 --> -5614623 
optimize edge weights:  -5614623 --> -5614623 
optimize base frequencies:  -5614623 --> -5614621 
optimize rate matrix:  -5614621 --> -5614621 
optimize edge weights:  -5614621 --> -5614621 
optimize base frequencies:  -5614621 --> -5614621 
optimize rate matrix:  -5614621 --> -5614621 
optimize edge weights:  -5614621 --> -5614621 
Generating phylogeny.. DONE!

Finished: 658 groups of orthologous from 10 isolates have been used in the alignment.
Returning an object of class "phylo" with 10tips and 8 nodes.
> p

Phylogenetic tree with 10 tips and 8 internal nodes.

Tip labels:
    C_fetus_subsp_testudinum_Sp3, C_fetus_subsp_venerealis_str_84-112, C_hyointestinalis_subsp_lawsonii_CCUG_27631, C_iguaniorum_str_RM11343, C_pinnipediorum_subsp_pinnipediorum_str_RM17262, H_bilis_str_AAQJH, ...
Node labels:
    100, 100, 100, 100, 100, 100, ...

Unrooted; includes branch lengths.
> class(p)
[1] "phylo"
> plot(p, type = 'unrooted', cex = 0.7, lab4ut = 'axial')

rplot

@giraola Sorry, I have been unable to find the time to get to the last check. I will try to find time this week.

@giraola thanks for reaching out. @kortschak @juanvillada thanks for your review and your feedback to @giraola's query.

@KlausVigo can you reply to @giraola in relation to what you think is preventing you from ticking the last boxes? Thanks

@giraola ,
all fine. Let me know if you need any help with the integration of phangorn or have suggestions to improve phangorn.
Regards,
Klaus

Thanks @KlausVigo for the fast reply.

Thanks @KlausVigo we will keep improving phylen.

Sorry for the delay. All accepted now.

Thanks @kortschak, @juanvillada and @KlausVigo for this constructive review process. @Kevin-Mattheus-Moerman, it seems every box has been checked by the three reviewers. What's next?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@giraola @iferres what is next is that I will congratulate you as I will now recommend that this work is accepted! :tada: :rocket: :robot:
At this point, can you please make an archive of the final reviewed software in Zenodo, or figshare, or another service, and update this thread with the DOI of the archive? Once we have that available we can process acceptance.

Thanks @kortschak, @juanvillada and @KlausVigo for your review efforts! :cake:

Hi, thanks everyone for your patience and suggestions.
Here's the doi for the archive 10.6084/m9.figshare.6223538

Arfon, over to you, we are all set. The authors say the DOI is 10.6084/m9.figshare.6223538. Although the link has .v1 in it see here: https://doi.org/10.6084/m9.figshare.6223538.v1, so make sure we'll use the correct one.

@whedon set 10.6084/m9.figshare.6223538.v1 as archive

OK. 10.6084/m9.figshare.6223538.v1 is the archive.

@kortschak, @juanvillada and @KlausVigo - many thanks for your reviews and to @Kevin-Mattheus-Moerman for editing this one โœจ

@iferres - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00593 :zap: :rocket: :boom:

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippet:

[![DOI](http://joss.theoj.org/papers/10.21105/joss.00593/status.svg)](https://doi.org/10.21105/joss.00593)

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Congratulations @iferres and @giraola ! :cake:

Was this page helpful?
0 / 5 - 0 ratings