-
-
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]: TipToft: detecting plasmids contained in uncorrected long read sequencing data #1021
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ctb, 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:
|
|
Hi everyone, I apologize for the delay in reviewing, but I've finally found a chance to give TipToft a proper review. I'm very impressed with TipToft and will likely eventually incorporate it into the software pipelines at where I work (public health lab). Thank you for developing it! I'm your target audience! We'll be doing nanopore sequencing soon and TipToft will be perfect. It's not so often that you find a piece of bioinformatics software that is well-documented, easy to install(!), easy to use, and runs fast like the way TipToft does. I've installed/ran TipToft via pip, bioconda, docker, and even bioconda via Ubuntu via Windows Subsystem for Linux on Win10. Each worked on the first time too 👍 I tested TipToft on the supplied test data (got the expected output), some of my own nanopore data, and some random nanopore data I pulled off of the SRA. It ID'd all replicons that I expected except for one. I'd previously used PlasmidFinder to ID an IncA/C2 replicon off of a nanopore-read-only assembly of a plasmid, but was unable to do so using those same raw reads and TipToft. I'm thinking this may because these aren't the highest quality nanopore reads and isn't the fault of TipToft. @andrewjpage if you'd like I can probably supply you with the reads and assembly (need to check with PI) if you're curious to figure out what happened. I do have one recommendation for the paper. I see the citation of the PlasmidFinder database in the repo, but I think it would be worth mentioning and citing the paper there too. I think that's all I can suggest for right now. Thanks for developing the tool and publishing it in an open-access way. It was fun to review! I'll leave an issue on github if I have any, and potentially a PR if there's any way I can improve it. That's an ACCEPT from me. |
Thanks @kapsakcj for your review contribution! 🎉 |
Sorry, it's taking too long. I'll manage to do it today.
…On Fri, Nov 9, 2018 at 7:05 AM Kevin Mattheus Moerman < ***@***.***> wrote:
@ctb <https://github.com/ctb>, @azneto <https://github.com/azneto>, can
you give an update as to when you are able to review this work? Thanks! 🤖
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1021 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFTWErmlzKUp-5bLodOT54fNL1GiuDLgks5utUVBgaJpZM4XcW5G>
.
|
@andrewjpage, congratulations! This is a very very clever method for finding the sequences of interests in datasets with considerable error rates. TipToft relies on the kmer matching approach to identify perfect hits, without the hassle of dealing with base quality and sequence assembly. I've ran the software a few times during the last weeks using both Fedora and Ubuntu. The software is well documented and runs really fast. Here are a few recommendations: Though the documentation for the end users is really good and comprehensive, the code itself is not so inviting. I tried to follow the algorithm step by step, but it was really hard since it doesn't follow the PEP8 (Style Guide for Python Code) and the PEP20 (The Zen of Python). I don't understand why would someone need to create aliases to functions or have duplicate functions in the code. For example: Fasta.sequence_to_kmers is an alias to Fasta.sequence_kmers There are variables and libraries that were created/loaded and never used (eg. time, operator, retcode). The software flake8 can find those and also check if the code is compatible with the PEP8. I also recommend using typing (https://docs.python.org/3/library/typing.html) for your functions to make sure the parameters and returns are correct and avoid implicitly passing of parameters using global variables. Maybe it's worth mentioning in the paper that if the user doesn't have at least 20x sequencing coverage, it's almost certain TipToft will not find all kmers. In anyways, the sequencing companies recommend at least 40x coverage to guarantee full error correction and rarely someone will have such problem. I agree with @hapsakcj regarding to the citation of PlasmidFinder and created an issue in the TipToft repo. Thanks for contributing to the open source software and bioinformatics communities! I'm glad I got the chance to learn about a new useful tool. That's an ACCEPT for me too. |
Thank you @kapsakcj and @azneto for the kind reviews and for taking the time to use and review the code. I have added in the PlasmidFinder reference and mentioned it in the text. I have reformatted the code and removed anything thats redundant or duplicated, so it now passes flake8 and pycodestyle without any errors. Thank you for the pointer to PEP20, its the first time I've come across it and I most definitely will follow it for all new Python projects. I'm relatively new to Python and have unfortunatly carried over bad habits from other languages.
In this instance I wanted to run a computationally intensive method exactly once and store the result. Happy to discuss this offline if there is a more appropriate pattern to use in Python.
Type hints look really awesome, thank for the suggestion. As this software currently works on 3.4 and above, adding type hints would increase this to 3.5 (probably 3.6 given the provisional status of the api). I think I will wait for it to stabilise before making the switch to minimise any potential barriers for researchers.
Yes you are absolutely right, in the real world you may not see all kmers if the coverage is too low. As its essentially an alignment based approach rather than de novo assembly it can detect a signal at a low coverage. I have added text to explain the depth of coverage. Unfortunatly there is not enough room in a JOSS paper to explain the algorithm indepth. |
@ctb, can you give an update as to when you are able to review this work? Thanks. |
@ctb, are you able to add your review comments? Thanks 🐰 |
It's thanksgiving week here in the US, and the kids are out of school.
Short-term prognosis is unclear.
|
@Kevin-Mattheus-Moerman Just a gentle reminder that this is still pending. Hope everyone had a nice holiday |
Thanks for the reminder and apologies for the delay with your submission. |
@ctb are you able to review this work at this point? Please let us know if you are no longer able to. Thanks. |
Review done! Agree with all the nice comments above - good work! I did not look at the code at all, since @azneto did such a nice job :) The only two issues that I saw that should be addressed are indicated in my unchecked boxes above --
but I did file another two issues that are merely suggestions,
Once the example data file issue is resolved (one way or another) I'm definitely an ACCEPT! |
Thanks @ctb! |
@andrewjpage please reply to @ctb 's comments. Please do add a CONTRIBUTING.md file (see also https://help.github.com/articles/setting-guidelines-for-repository-contributors/). @ctb feel free to tick that version box. We will update the final version number upon acceptance (future review issues will not have the version box any more as versions often move on). |
Thanks @ctb for taking the time to review the software. I have resolved all the issues raised, adding a CITATION.cff file, added back the example data file, adding a CONTRIBUTING.md and a CoC. Thanks @Kevin-Mattheus-Moerman for editing. |
@ctb thanks again for your review work, are you happy with how @andrewjpage has responded to your comments? |
yes!
|
@ctb great, please tick the remaining box. Thanks for your help! |
done. |
@whedon generate pdf |
|
@whedon check references |
@whedon set 10.5281/zenodo.2561192 as archive |
OK. 10.5281/zenodo.2561192 is the archive. |
@whedon set v1.0.1 as version |
OK. v1.0.1 is the version. |
Thanks @Kevin-Mattheus-Moerman for editing, and @ctb & @kapsakcj & @azneto for reviewing! |
@whedon accept |
|
PDF failed to compile for issue #1021 with the following error: % Total % Received % Xferd Average Speed Time Time Time Current 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 |
|
I suspect an error in the whedon's accept processing. I will check by trying to generate the pdf by itself |
@whedon generate pdf |
|
👋 @arfon - please check this and see what's off in whedon's accept processing. |
👋 @arfon - also, the archive link on the left of the PDF itself doesn't seem to be correct. |
@whedon accept |
|
|
Check final proof 👉 openjournals/joss-papers#536 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#536, then you can now move forward with accepting the submission by compiling again with the flag
|
👋 @arfon - my issues seem to be resolved - should I go ahead and finalize the accept? |
Yes, please do. Sometimes Whedon fails to compile the PDF properly - this bug is something I've never managed to replicate. As for the faulty PDF, |
@whedon accept deposit=true |
|
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? notify your editorial technical team... |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
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: @andrewjpage (Andrew Page)
Repository: https://github.com/andrewjpage/tiptoft
Version: v1.0.1
Editor: @Kevin-Mattheus-Moerman
Reviewer: @ctb, @kapsakcj, @azneto
Archive: 10.5281/zenodo.2561192
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
@ctb & @kapsakcj & @azneto, 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.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @ctb
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @kapsakcj
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @azneto
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: