-
-
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]: cerebra: A tool for fast and accurate summarizing of variant calling format (VCF) files #2432
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @betteridiot, @afrubin it looks like you're currently assigned to review this paper 🎉. Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. ⭐ 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:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Hi @betteridiot, @afrubin!, this is the review issue, please accept the invitation if you haven't done it so you can start the review here. @lincoln-harris, can you work on adding DOI to your references? Thanks all! |
sure! i can do that |
It looks like the references aren't formatting properly, possibly due to missing commas in the author names in the .bib file. @lincoln-harris would you please have a look? |
ReviewThis certainly looks like a useful and performant tool for transforming VCF files into a format that is easier to work with and enables researchers to make biological inferences. I've opened some issues (referenced above) but thought it better to put high-level feedback and comments on the paper in this thread. I have not run the software yet, but plan to follow the updated installation instructions and run some test data after czbiohub-sf/cerebra#83 is addressed. TestsTest coverage is low (81%) and excludes quite a few cases that seem like they might come up when used on diverse data. Error messages/logging may also need to be improved. For example, this section is not covered by tests and includes output to stdout with limited context: Several failure conditions (e.g. https://github.com/czbiohub/cerebra/blob/3553f707290e57440ca9c2ab617667ab7cc4323a/cerebra/germline_filter.py#L114-L115) are not covered by tests, although adding test cases for these ought to be trivial. The multiprocessor use case for DocumentationMost usable documentation is in the README. This would be easier to navigate if it were split into multiple files, but another option is to focus on improving the README's content and structure, and dropping the docs/ directory altogether. Many parts of the documentation seem to be lacking updates and consistency. For example, the instructions in docs/installation.rst are incomplete and different to what's in the README. The docs/authors.rst references a top-level AUTHORS.rst, which doesn't exist (although there is an AUTHORS.md that doesn't include all the contributors). There are similar issues affecting other files. The repository is in need of a "polish pass" to clean up these kinds of issues and remove unnecessary files (such as .editorconfig in the top level). PaperThe motivation section should make it clear that an intended use case is single-cell RNA-seq data. This isn't introduced until the The authors also use the term "functional predictions" to mean predicted amino-acid changes. This package does not attempt to predict the "functional consequences of the variant", which is a much more ambitious problem. The authors should avoid using the term "functional" throughout the text and focus on what the tool is actually doing - inferring amino acid consequences given variant calls and gene annotations. There are also numerous missing citations, across packages used (ncls is not cited despite having citation information in its GitHub README), important genomic formats (VCF and HGVS are not cited), and databases (COSMIC is not cited). I should emphasize that this is a partial list of missing citations. Regarding the use of databases, the authors make the nonsensical claim that "variants that are not found in the COSMIC database [are] less likely to be pathogenic". While there are good reasons to filter for known variants (e.g. focusing on clinical actionability), this isn't one. COSMIC includes a tiny fraction of pathogenic variants, most of which are rare. For variant discovery, users may want to filter for variants that are not found in COSMIC, and the authors should consider implementing this as an option. This would be especially useful if support for variant databases like gnomAD is added in the future. Also, the paper mentions dbSNP filtering but it doesn't look like this is implemented based on the documentation. One small nomenclature issue is that the transcripts in Fig 2 are referred to as "spliceforms" in the text but when alternative start sites are included (like t3) the more general term "isoforms" should be used. SummaryThis is a useful and interesting tool that would be appropriate for JOSS but additional work is needed across the project before it fulfills the criteria. |
thanks for the comments, these are currently being addressed with czbiohub-sf/cerebra#84 |
@lpantano For some reason, GitHub is not allowing me to check the boxes (like in previous reviews). I may need another invitation. |
@whedon re-invite @betteridiot as reviewer |
OK, the reviewer has been re-invited. @betteridiot please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations |
Thank you. Apparently it was just a browser extension that was causing the problem. |
For transparency's sake, I am still actively working on this review. However, I have a thesis committee meeting this week, and will be not be able to get back to the review process until then. Sorry for the delay. |
the majority of reviewer comments have been addressed with the latest release. |
@lincoln-harris I have a few additional comments on the writing in the germline-filter section of the paper and the README. A more typical phrasing from cancer genomics would be "tumor vs. normal" rather than "tumor/pathogenic vs control." The text says that germline-filter can be used "so as to not bias the results by including non-pathogenic variants." While I understand the motivation for this, you can't say that germline variants are non-pathogenic - they may well be strongly pathogenic for another disease! Instead of "control and the experimental tissue" the text should stick with the word "sample" (and tumor/normal) for clarity. |
Hi @lincoln-harris, Any update on the latest @afrubin's comment? @betteridiot, any update for this review? Thanks! |
OK. 10.5281/zenodo.4050557 is the archive. |
@whedon accept |
|
PDF failed to compile for issue #2432 with the following error: /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/lib/whedon/bibtex_parser.rb:77:in |
ops, @lincoln-harris do you think is related to your paper? did you change something from the last time it compiled here? |
@whedon accept |
|
|
PDF failed to compile for issue #2432 with the following error: /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/lib/whedon/bibtex_parser.rb:77:in |
@lincoln-harris I am helping to process the acceptance of this work in JOSS. Can you work on the points below?
with
|
@openjournals/dev I identified the invalid DOI but am unsure if this is the reason the paper does not compile for |
@Kevin-Mattheus-Moerman done, hopefully that takes care of it. |
@whedon accept |
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#1770 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1770, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 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:
|
Congratulations @lincoln-harris! Thanks for your review efforts @betteridiot, @afrubin, also thank you @lpantano for editing! |
awesome!! thanks @betteridiot, @afrubin and @lpantano!! I learned a ton through the review process |
Submitting author: @lincoln-harris (Lincoln Harris)
Repository: https://github.com/czbiohub/cerebra
Version: v1.2.0
Editor: @lpantano
Reviewer: @betteridiot, @afrubin
Archive: 10.5281/zenodo.4050557
Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.
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) by leaving comments 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
@betteridiot & @afrubin, 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.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @lpantano know.
✨ Please try and complete your review in the next six weeks ✨
Review checklist for @betteridiot
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @afrubin
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: