-
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
plot_phylo #160
Comments
Hello there, and welcome to the pyOpenSci community-- we are so happy you are here! Just wanted to let you know we have seen your submission and will get back to you with some initial checks soon. 👾 |
Editor in Chief checksHi there! Thank you for submitting your package for pyOpenSci Please check our Python packaging guide for more information on the elements
Editor commentsThank you again for submitting Blockers to beginning review process:
Some smaller nits:
If you have any questions or need any support with these tasks, pyOpenSci community would love to help out! The best places to get in touch are via discourse or Slack--let me know if you need an invite. And, of course, you can also reply here (but then you will be restricted to just my opinions 😂)! |
Following up, @ctb will the editor leading the review for |
Hello all! Checking in here. @KatyBrown are you able to give an update on the status of a documentation website? We won't be able to start a review without that put up. If you need any support, please let us know! |
Hi @isabelizimm , sorry, I got caught in other projects but I'm working on moving the documentation the readthedocs - it should be ready in another couple of days. Sorry to be slow! Thanks for checking in. |
No worries at all--thank you for the update! 🌻 If you can give us a heads up in this issue once those docs are up, then we can get started on the review. |
I am alive! Just let me know when to start finding reviewers :) |
Hi @isabelizimm, very sorry for the long delay, I've now made the changes you asked for. The current version of the tool is now v0.1.9 - I've edited this in my original post. Specifically I have:
Please let me know if there's anything else I need to do, otherwise I think I'm now ready to proceed to the next step :) Thank you! |
No worries-- with those all updated, I can hand everything over to @ctb to look for reviewers! |
Apologies for delay - started beating the bushes for some reviewers. In the meantime, filed a few minor issues myself ;). The figures are lovely!! |
@ctb thanks very much! I think I've addressed the issues you raised. |
apologies for my long delay - have one reviewer locked in, reach out to some more today. |
Editor response to review:Editor comments👋 Hi @ammaraziz and @jkanche! Thank you for volunteering to review Please fill out our pre-review surveyBefore beginning your review, please fill out our pre-review survey. This helps us improve all aspects of our review and better understand our community. No personal data will be shared from this survey - it will only be used in an aggregated format by our Executive Director to improve our processes and programs.
The following resources will help you complete your review:
Please get in touch with any questions or concerns! Your review is due: July 22nd, 2024!Reviewers: @ammaraziz and @jkanche |
Apologies for the delay, I plan to finish my review this week. |
thanks for checking in! @ammaraziz, need anything from me? |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Readme file requirements
The README should include, from top to bottom:
NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)
UsabilityReviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Functionality
Review Comments
I cloned the repo and installed it in edit mode: ⋊> ~/P/p/t/plot_phylo on main ◦ pytest --cov=plot_phylo (reviews) 16:09:15
=========================================== test session starts ============================================
platform darwin -- Python 3.11.9, pytest-8.2.2, pluggy-1.5.0
rootdir: /Users/kancherj/Projects/public/trash/plot_phylo
configfile: pyproject.toml
plugins: cov-5.0.0
collected 0 items / 3 errors
/opt/homebrew/lib/python3.11/site-packages/coverage/control.py:888: CoverageWarning: No data was collected. (no-data-collected)
self._warn("No data was collected.", slug="no-data-collected")
================================================== ERRORS ==================================================
______________________________ ERROR collecting tests/test_get_boxes_data.py _______________________________
ImportError while importing test module '/Users/kancherj/Projects/public/trash/plot_phylo/tests/test_get_boxes_data.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_get_boxes_data.py:2: in <module>
import matplotlib.pyplot as plt
E ModuleNotFoundError: No module named 'matplotlib'
________________________________ ERROR collecting tests/test_plot_phylo.py _________________________________
ImportError while importing test module '/Users/kancherj/Projects/public/trash/plot_phylo/tests/test_plot_phylo.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_plot_phylo.py:2: in <module>
import matplotlib.pyplot as plt
E ModuleNotFoundError: No module named 'matplotlib'
______________________________ ERROR collecting tests/test_plot_phylo_data.py ______________________________
ImportError while importing test module '/Users/kancherj/Projects/public/trash/plot_phylo/tests/test_plot_phylo_data.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
tests/test_plot_phylo_data.py:2: in <module>
import plot_phylo
E ModuleNotFoundError: No module named 'plot_phylo'
---------- coverage: platform darwin, python 3.11.9-final-0 ----------
Name Stmts Miss Cover Missing
--------------------------------------------------------
plot_phylo/plot_phylo.py 144 144 0% 2-586
--------------------------------------------------------
TOTAL 144 144 0%
========================================= short test summary info ==========================================
ERROR tests/test_get_boxes_data.py
ERROR tests/test_plot_phylo.py
ERROR tests/test_plot_phylo_data.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 3 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Overall, the package provides an intuitive and easy way to integrate phylogeny visualizations with matplotlib based workflows. Let me know if you have any questions. |
Hi @ctb and @KatyBrown, please accept my apologies. I've been stretched thin over the last few months. I will make time for reviewing this week if that is acceptable? |
as have we all! no worries and I really appreciate your willingness to review!
please and thank you! |
Package Review
DocumentationThe package includes all the following forms of documentation:
Readme file requirements
The README should include, from top to bottom:
NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)
UsabilityReviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Functionality
For packages also submitting to JOSS
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted. The package contains a
Final approval (post-review)
Estimated hours spent reviewing: Review CommentsGreat package, works nicely and the provided example is a great start. The power of matplotlib means it's possible to make great plots.
But I get an error:
Testing works if you run just
Good job on the great package. |
thanks @ammaraziz and @jkanche!! @KatyBrown please take a detailed look at the above, and I will do the same! |
@jkanche https://pyscaffold.org/en/stable/ looks fascinating! |
@jkanche and @ammaraziz, thank you so much for your reviews!! @KatyBrown, I've extracted the unchecked bits of the reviews below -- please see bottom for my thoughts! pyopensci -- summary of reviews by CTBPackage review by @jkanche:DocumentationThe package includes all the following forms of documentation:
The README should include, from top to bottom:
UsabilityIn general please consider whether:
Functionality
Review Comments
I cloned the repo and installed it in edit mode: (errors elided)
CTB comment: you could put these in the issue tracker, if you didn't want to implement them now :) Overall, the package provides an intuitive and easy way to integrate phylogeny visualizations with matplotlib based workflows. Let me know if you have any questions. Package Review by @ammarazizDocumentationThe README should include, from top to bottom:
Functionality
Review CommentsGreat package, works nicely and the provided example is a great start. The power of matplotlib means it's possible to make great plots.
But I get an error:
Testing works if you run just (elided)
Good job on the great package. Editor comments from CTBThe three major suggestions from the reviews that stand out are -
The rest all seem like fairly quick add-this/adjust-that changes. Please let me know if you have thoughts or questions! |
Hi @ctb @jkanche @ammaraziz thanks so much for looking at this and sorry for my slow response. I will look at your comments over the next few days. One quick thing, I do have CI testing which runs my test suite - the workflow is here https://github.com/KatyBrown/plot_phylo/actions/workflows/main.yml. I'm not sure if they are not showing up somehow. Thanks again and I'll address everything else asap. |
Hi everyone, apologies for my extremely slow response to this. I really appreciate all the feedback on my package. |
Sorry about that, I’ve added new instructions to the manual - which hopefully now work.
I have now added these to the main page.
I’ve now added publishing automatically to pip as a GitHub workflow - I’ll also look into this package as it sounds useful!
It is now included
I’ve added these two as enhancements in the issue tracker - as I have a few other enhancements I’d like to make at the same time and I don't want to delay this even further.
Thank you very much! |
Thank you! I appreciate it and I’m glad it works!
Apologies for this, I’ve updated the manual - with instructions which hopefully now work.
I’ve added a QuickStart guide to the README.md along with an additional link to the full documentation, so it’s easier to find.
I already have this example here but I’ve tried to make the manual easier to find from the README.
I was already using GitHub actions for CI testing here, I’ve added details of this to the README.md and CONTRIBUTING.md files so it is easier to find. I’ve also now added publishing automatically to pip as a GitHub workflow
Thank you - I’ve added this now to the README
Thank you very much! |
I think it was not very clear where to find my documentation, I’ve added additional links. If there is still not enough detail please let me know.
I’ve updated the manual - with instructions which hopefully now work.
Thank you. I was already using GitHub actions for CI testing here, I’ve added details of this to the README.md and CONTRIBUTING.md files so it is easier to find. I’ve also now added publishing automatically to pip as a GitHub workflow
Thank you! @ctb I’ve also done the following to address additional things on the checklist:
|
Thanks very much everyone for the feedback and please let me know if there's any more I can do! |
Hi @ctb, @ @ammaraziz @jkanche I was just wondering if there's any progress on this? Thanks very much. |
Not sure what needs to be done from the reviewers end? @ctb |
Hi @KatyBrown apologies for the delay, busy December! I submitted a small PR to fix a few things in the README (see KatyBrown/plot_phylo#8) but it all looks good to me! If @ammaraziz and @jkanche could update their checkboxes that would be ideal, but I've verified most of them myself. I'll figure out what needs to be done to fully accept when I'm back in a week. Happy New Year! |
This looks good to me. Optionally, you may also create a citation.cff file in the repo. GitHub add a new menu item on the right sidebar to quickly copy the citation. But otherwise looks good. Thank you for all the work and Happy new year! |
Looks good. Updated check list. |
@jkanche @ammaraziz @ctb Thanks very much! |
Thanks - I've added the citation.cff file. |
🎉 plot-phylo has been approved by pyOpenSci! Thank you @KatyBrown for submitting and many thanks to @jkanche @ammaraziz for reviewing this package! 😸 Author Wrap Up TasksThere are a few things left to do to wrap up this submission:
Editor Final ChecksPlease complete the final steps to wrap up this review. Editor, please do the following:
If you have any feedback for us about the review process please feel free to share it here. We are always looking to improve our process and documentation in the peer-review-guide. |
dear @jkanche @ammaraziz @KatyBrown if you could fill out the post-review survey (it's very brief!) we would appreciate it! also, please let me know if you would like to join the pyopensci slack, and if so drop me an e-mail with your preferred invite e-mail at [email protected]. last but not least, @KatyBrown - we wanted to invite you to write a blog post (totally optional) on your package for us to promote your work! Here are a few examples of other blog posts: and here is a markdown example that you could use as a guide when creating your post. You could write a tutorial, too! If you are too busy for this no worries. But if you have time - we'd love to spread the word about your package! |
Submitting Author: Katy Brown (@KatyBrown)
All current maintainers: Katy Brown (@KatyBrown)
Package Name: plot_phylo
One-Line Description of Package: A Python package to plot a phylogenetic tree on an existing Matplotlib axis.
Repository Link: https://github.com/KatyBrown/plot_phylo
Version submitted: 1.9.0
EIC: @isabelizimm
Editor: @ctb
Reviewer 1: @ammaraziz
Reviewer 2: @jkanche
Archive: TBD
JOSS DOI: TBD
Version accepted: TBD
Date accepted (month/day/year): TBD
Code of Conduct & Commitment to Maintain Package
Description
plot_phylo
is a Python package which allows the user to plot phylogenetic trees onto an existing matplotlib axis. This means annotations can be added using matplotlib functionality and figures including phylogenies can be included in automated figure generation.Scope
Please indicate which category or categories.
Check out our package scope page to learn more about our
scope. (If you are unsure of which category you fit, we suggest you make a pre-submission inquiry):
Domain Specific & Community Partnerships
Community Partnerships
If your package is associated with an
existing community please check below:
For all submissions, explain how the and why the package falls under the categories you indicated above. In your explanation, please address the following points (briefly, 1-2 sentences for each):
Who is the target audience and what are scientific applications of this package?
Researchers working with phylogenetic tree data, a common data format for evolutionary and functional analysis of biological data.
Are there other Python packages that accomplish the same thing? If so, how does yours differ?
plot_phylo depends on the functionality of ETE Toolkit, a widely used visualisation tool. ETE provides excellent tree visualisations but these act as a stand-alone image, which can't be incorporated into another programmatically generated figure except as an uneditable embedded image. plot_phylo trees are subplots of open matplotlib figures, so custom annotations can be added, aspects of the image can be updated and other subplots can show additional data.
If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or
@tag
the editor you contacted:Technical checks
For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:
Publication Options
JOSS Checks
paper.md
matching JOSS's requirements with a high-level description in the package root or ininst/
.Note: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.
Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?
This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.
Confirm each of the following by checking the box.
Please fill out our survey
submission and improve our peer review process. We will also ask our reviewers
and editors to fill this out.
P.S. Have feedback/comments about our review process? Leave a comment here
Editor and Review Templates
The editor template can be found here.
The review template can be found here.
Footnotes
Please fill out a pre-submission inquiry before submitting a data visualization package. ↩
The text was updated successfully, but these errors were encountered: