-
-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVIEW]: Phylen: automatic phylogenetic reconstruction using the EggNOG database #593
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 🎉. ⭐ Important ⭐ 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:
For a list of things I can do to help you, just type:
|
|
@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). |
@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 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 |
@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 @Kevin-Mattheus-Moerman Can you clarify the interpretation of this question? |
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:
Cheers, |
I reinstalled and tried > 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') |
@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 , |
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 |
|
@giraola @iferres what is next is that I will congratulate you as I will now recommend that this work is accepted! 🎉 🚀 🤖 |
Thanks @kortschak, @juanvillada and @KlausVigo for your review efforts! 🍰 |
Hi, thanks everyone for your patience and suggestions. |
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 ⚡ 🚀 💥 |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippet:
This is how it will look in your documentation: 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:
|
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 badge code:
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:
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
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?REVIEWER 2
@kortschak, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
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
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?REVIEWER 3
@KlausVigo, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
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
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?The text was updated successfully, but these errors were encountered: