-
-
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]: pymatgen-analysis-defects: A Python package for analyzing point defects in crystalline materials #5941
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 |
|
Reminder to reviewers @ireaml and @atimmins7 to get your reviews started when you have a chance! If you have any questions about the process, (read the first comment in this issue and then) don't hesitate to ask! Also note that you should feel free to file issues (or even PR's) in the project repo as you work on your review; please link to this issue for easy tracking if you do so. Thanks! |
Checking in again with @ireaml and @atimmins7 – hoping you can start reviews soon! |
Review checklist for @ireamlConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Review checklist for @atimmins7Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hi @rkurchin, I've finished my review. This is a very high-quality code, with a strong motivation, easy to use and very well documented. It will be very useful for the defect community and I would be very happy to recommend publication in JOSS. For the author, @jmmshn, these are my suggestions: Regarding the paperLine 10: Ref 2 doesn’t display correctly. Further, it would be good to also cite the other existing packages that have been developed to pre- or post-process defect calculations, including:
Minor typos:Line 16: “most users of a defect analysis packages” -> “most users of defect analysis packages” Regarding the docs
Small typos:
Overall, really excellent! :) |
(noting that I've pinged @atimmins7 separately about his review; I know he has started working on it!) |
@rkurchin @jmmshn - The pymatgen-analysis-defects package is well explained by the document, the provided tutorial clearly demonstrates high-level functionality, and the code is clear and easy to follow. As a note, a number of the features rely heavily on VASP, which I do not use or have access to. As a result, I have relied on the example files in the tutorial to validate that functionality is as stated. Where possible, I used Quantum Espresso to test package functionality. Some typos have already been highlighted and have not been repeated here. The notes below are minor, mostly about improving phrasing or code comments to improve clarity. PaperMinor citation error in line 10, "Huang et al. (2022)," DocumentationI did not see reference to community guidelines as defined the rubric. TutorialQuickstart
Defining Defects
Defect Finder
Shockley-Read-Hall Recombination
Photo Conductivity
Formation Energy Diagrams
Freysoldt Correction
Code ReviewThe code is clear and well documented. It was possible to interpret the purpose of each class/function from the code itself and the included comments. So minor fixes have been included to improve the clarity of the logic. Some internal functions do not have explanations. This may be intentional. However in case it was a mistake, any missing explanations have been noted below. ccd.py
core.py
finder.py
generators.py
recombination.py
supercells.py
thermo.py
utils.py
Corrections > freystoldt.py
Plotting > optics.py
|
Thank you @atimmins7 @ireaml and @rkurchin, I will go over all the suggestions carefully and incorporate the changes. |
Hi @atimmins7 @ireaml and @rkurchin, |
Well noted! Thank you for letting me know. @jmmshn @rkurchin - I have reviewed the changes and only noticed a few small aesthetic typos. As a result, I support final publication and have updated my checklist accordingly. Although these are not blocking items, the following typos might be fixed to improve clarity.
|
Sorry for the delay. The changes look good! I've updated my checklist accordingly and am happy to support publishing. Although not a blocking issue, I would suggest updating https://materialsproject.github.io/pymatgen-analysis-defects/content/nonradiative.html#potential-energy-surface to specify that this assumes the harmonic approximation, with maybe:
|
Post-Review Checklist for Editor and AuthorsAdditional Author Tasks After Review is Complete
Editor Tasks Prior to Acceptance
|
Thanks again to reviewers @ireaml and @atimmins7 for your work on this! @jmmshn, please send me the information requested in the checklist above via comment here and I'll send any editorial comments on the manuscript itself shortly. We're getting close to finished! :) |
OK, I updated the title on the archive. |
Can you also update the author list to match? |
Done |
@editorialbot set 10.5281/zenodo.6994437 as archive |
Done! archive is now 10.5281/zenodo.6994437 |
@editorialbot set 2024.1.12 as version |
Done! version is now 2024.1.12 |
@editorialbot recommend-accept |
|
|
👋 @openjournals/pe-eics, this paper is ready to be accepted and published. Check final proof 👉📄 Download article If the paper PDF and the deposit XML files look good in openjournals/joss-papers#4911, then you can now move forward with accepting the submission by compiling again with the command |
Hi @jmmshn, I made some minor edits to the paper in materialsproject/pymatgen-analysis-defects#169, can you review and merge that? Also, please add the DOI 10.1103/RevModPhys.86.253 to reference Freysoldt et al. |
@editorialbot generate pdf |
sorry I did push the change to the DOI. |
@editorialbot generate pdf |
Thanks @jmmshn, this looks good to publish now. |
@editorialbot accept |
|
Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository. If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file. You can copy the contents for your CITATION.cff file here: CITATION.cff
If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation. |
🐘🐘🐘 👉 Toot 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 @jmmshn on your article's publication in JOSS! Please consider signing up as a reviewer if you haven't already. Many thanks to @ireaml and @atimmins7 for reviewing this, and @rkurchin for editing. |
🎉🎉🎉 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! The 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: @jmmshn (Jimmy-Xuan Shen)
Repository: https://github.com/materialsproject/pymatgen-analysis-defects
Branch with paper.md (empty if default branch):
Version: 2024.1.12
Editor: @rkurchin
Reviewers: @ireaml, @atimmins7
Archive: 10.5281/zenodo.6994437
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
@ireaml & @atimmins7, 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 @rkurchin 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 @ireaml
📝 Checklist for @atimmins7
The text was updated successfully, but these errors were encountered: