-
-
Notifications
You must be signed in to change notification settings - Fork 36
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]: PhyloX: A Python package for complete phylogenetic network workflows #6427
Comments
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. 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:
|
|
Wordcount for |
|
Review checklist for @abhishektiwariConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@RemieJanssen Please see my initial comments. Example code on documentation landing page did not run for me. Otherwise I was able run test scripts and create my own scripts without any issues. A few minor typos in paper reported here. I also think first line of paper's |
@bgyori - I hope that everything's ok. please feel free to create your review checklist. Please let me know if you have questions. |
@RemieJanssen - do you have any questions about the comments from & suggestions from @abhishektiwari ? |
Review checklist for @bgyoriConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@bgyori - just wanted to check to see how the review is going; do you have any questions or need assistance as you look over things? Thank you! |
@RemieJanssen - did you have a chance to update the software in response to the comments from @abhishektiwari ? Please let us know if you need any clarifications. Thank you! |
I have now completed my initial review, see checklist above. Below, I provide further context for checkboxes that I didn't check. General checks
Functionality
Documentation
Software paper
|
@fboehm I have updated the software to fix the main example in the documentation. It's still only in the main branch, so it's not released yet. I'll do that once I have updated the paper, and possibly the documentation, if that is required as well. @bgyori Thank you for the explanation of the non-checked items. I will go through those and respond and fix what I can. @fboehm What is generally the way forward in this phase? I'm quite used to the clearly delineated phases of a more conventional review process, this seems somewhat more fluid. I'm sorry I'm taking so long, I unfortunately do not have as much time to work on this as I would want to. |
@editorialbot generate pdf |
Please see more feedback Quality of writingSome parts need rewriting like related packages (State of the field). It can be more crisp, bit shorter (as suggested by other reviewer), and objective. I will avoid using weasel phrases like fit that bill to a certain extent, and be more specific. I felt comparison on the documentation page did a better job by calling out the specific differences between PhyloX and alternatives.
I also suggest highlighting some of the key features/functionalities of the PhyloX software in paper. Again, I think documentation page does a better job describing key features of PhyloX. Example usageThanks for fixing the example. I would love to see one or two Jupyter notebook based example use cases. But this is not a blocker from my side. API documentationModule functions for the health generator are missing please fix RST tree. |
@RemieJanssen - do you have questions about next steps? Thanks again! |
Sorry that I missed your earlier messsge. Yes, compared to some review processes, that of Joss may feel different. It is essentially an iterative process between you and the reviewers. Once they are able to check all boxes, their roles are complete. As author, you just need to respond to their suggestions. If you disagree with a suggestion, please discuss it in this thread. |
I'm just checking back again, @RemieJanssen have you had a chance to look into the paper improvements / documentation improvements that @abhishektiwari and I suggested? I don't see any recent changes since my PR was merged with some typo fixes, though I might have missed something. |
Ah! That does answer my question, thank you :) I finally have some time again, so I'll try to get as far as possible responding to all the comments this week. |
I don't think I can really argue with you here, most of the package was written by me, and compiled over a short period of time. However, it's not really true that it was written over a limited period. The package is a compilation of most code was written during the 4 years of my PhD and the few years after (and not even all by me, but also by co-authors). This package is an attempt to not let all that effort go to waste because of our unFAIR code practices. |
@RemieJanssen @bgyori - can you elaborate on this point? Why does it appear to @bgyori that things were written over a short period while @RemieJanssen says otherwise? Is @bgyori relying on git commit times, while @RemieJanssen and co-workers perhaps didn't use git while writing? |
Based on @RemieJanssen's comments, I understand that the code was written over a several years but was consolidated into this integrated repository later, over a shorter period of time. This makes sense to me, and I definitely see the value in turning fragmented code into an integrated and reusable package. |
That is correct. If you want to see proof of this, take a look at the second commit: RemieJanssen/PhyloX@2b412c8 |
I tried to re-balance the paper as @bgyori suggested, in this PR: RemieJanssen/PhyloX#52 |
@editorialbot generate pdf |
@bgyori About the Functionality documentation. Did you have any problems navigating the docs besides the issue of missing docstrings?
I think that is all of them (except some helper functions for API methods), please let me know if you find any more |
@bgyori - do you have any feedback for @RemieJanssen in response to the latest changes? |
As far as I understand, @RemieJanssen is working on some documentation updates still (is that right?), I will reassess everything once that is wrapped up. |
@bgyori I have filled in the missing docstrings wherever I could find them, and I have improved (hopefully) the navigability of the docs: https://phylox.readthedocs.io/en/latest/ If you find anything else, please let me know :) |
@abhishektiwari @bgyori @RemieJanssen - how are the reviews going? Has @RemieJanssen addressed your concerns? Or is there more revision needed? |
@fboehm I have marked my review completed. Thanks authors for updating documentation and paper. |
Thank you for the helpful review, @abhishektiwari ! |
@bgyori - have you had a chance to review the latest revisions from @RemieJanssen ? |
@bgyori - I hope that things are going ok. Have you had a chance to review changes from @RemieJanssen ? DO you have any questions on how to proceed? |
I will aim to finish my review before the end of this week. |
Thanks @RemieJanssen, I think all aspects of the software and paper have been improved substantially and I checked all the review boxes. I submitted a PR with a couple of small typo fixes to the paper. |
Thanks, @bgyori ! @RemieJanssen - please let me know when you've addressed the PR from @bgyori . Thanks again! |
Thanks @bgyori and @abhishektiwari for the helpful reviews, and @fboehm for the smooth process! The last thing I'd like to fix is some metadata. |
@editorialbot list commands |
I'm sorry human, I don't understand that. You can see what commands I support by typing:
|
@editorialbot commands |
Hello @fboehm, here are the things you can ask me to do:
|
Post-Review Checklist for Editor and AuthorsAdditional Author Tasks After Review is Complete
Editor Tasks Prior to Acceptance
|
Submitting author: @RemieJanssen (Remie Janssen)
Repository: https://github.com/RemieJanssen/phylox
Branch with paper.md (empty if default branch):
Version: v1.0.3
Editor: @fboehm
Reviewers: @abhishektiwari, @bgyori
Archive: Pending
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
@abhishektiwari & @bgyori, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @fboehm know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @abhishektiwari
📝 Checklist for @bgyori
The text was updated successfully, but these errors were encountered: