-
-
Notifications
You must be signed in to change notification settings - Fork 40
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]: pyscal : A python module for structural analysis of atomic environments #1824
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @lucydot, @bocklund it looks like you're currently assigned to review 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:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
I just finished my first pass on the review. I have a couple outstanding items to address: Installation: Does installation proceed as outlined in the documentation? - I opened pyscal/pyscal#31. After this is closed, I can check this box. State of the field: Do the authors describe how this software compares to other commonly-used packages - This is missing from the documentation and the JOSS paper. |
@bocklund thanks a lot for the quick review. I will work on the issues soon. Just a quick clarification regarding the State of the field part. I know ideally it should be on the paper, but do you think adding it to documentation will suffice? |
Hi @srmnitc - I just finished my first look over. The documentation is really nice, and I like the use of binder for someone who wants to get a quick feel for the code. Installation: I have the same issue as reported in pyscal/pyscal#31 when installing through conda. I can confirm installation runs smoothly using State of the field: I don't want to call whether this is ok in the broader documentation, perhaps the editor @melissawm could clarify? It's listed under Documentation: I have a few suggestions regarding the documentation; these are just ideas that would not block paper acceptance. I've raised an issue in the repo: pyscal/pyscal#34. |
@lucydot thanks for the review and the comments. I think I can add few sentences in the paper about other codes. There are few other python modules, but none of them seems to be maintained/documented, and lacks all of the features this module has. Bigger tools like LAMMPS, for example, can also do this, but the target is different. We are looking at mostly post-processing of data. But I will add few sentences in the paper regarding this. Once again, thanks for the quick review. |
@lucydot @bocklund I have added both numpy and matplotlib as dependencies. pyscal/pyscal#31 has been closed. |
@bocklund @lucydot @melissawm, About State of the field, commit pyscal/pyscal@661b677 has an updated version of paper that references another python module has some overlapping functionality. This is the only one I could find with documentation and a doi. LAMMPS and PLUMED can also calculate the basic version of Steinhardt parameters, but the primary focus is completely different, so I think they will not fall in the same category as this module. |
@srmnitc - fab, I can confirm that tests pass after install via conda. I'm also happy to tick off state of the field. I'd like another day or two to play around with the code (rather than just run through the examples given) before signing off - Lucy |
@lucydot thanks! |
With the latest changes, I'm recommending this submission for acceptance. Nice work, @srmnitc! @melissawm |
@bocklund thanks for the review and suggestions! |
@melissawm I have created the archive. The DOI is 10.5281/zenodo.3522376 |
@srmnitc good work, it was a pleasure to review and happy to recommend for acceptance |
@whedon generate pdf |
|
@melissawm PDF looks to be correct. Thanks! |
@srmnitc A few points on the citations:
|
Check final proof 👉 openjournals/joss-papers#1080 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1080, then you can now move forward with accepting the submission by compiling again with the flag
|
sorry, I didn't check the bib entries at the same time - can you also merge pyscal/pyscal#39 ? |
@danielskatz merged. thanks! |
@whedon generate pdf |
|
@whedon accept |
|
|
Check final proof 👉 openjournals/joss-papers#1081 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1081, 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... |
Thanks to @lucydot & @bocklund for reviewing and @melissawm 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! 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:
|
@danielskatz thanks a lot ! |
@arfon Hi Arfon, I am the author of the package in this review, which is now published. I had a quick question. We would like to change the license of the package from GNU GPL to BSD. It still remains open source, but it is for integration into a larger project. Is this okay from the perspective of JOSS? Sorry about the trouble! |
@srmnitc - this is fine with us. Thanks for checking! |
Submitting author: @srmnitc (Sarath Menon)
Repository: https://github.com/srmnitc/pyscal
Version: v2.0.1
Editor: @melissawm
Reviewer: @lucydot, @bocklund
Archive: 10.5281/zenodo.3522376
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
@lucydot & @bocklund, 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 @melissawm know.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @lucydot
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @bocklund
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: