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]: Pyrokinetics - A Python library to standardise gyrokinetic analysis #5866
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 |
hi @the-rccg @rogeriojorge please let me know when and if you'll be able to start your review here, and if you have any questions or concerns as well. thanks! |
Review checklist for @rogeriojorgeConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
This is the start of the review of the code Pyrokinetics - A Python library to standardise gyrokinetic analysis Below, I describe the process of download, installation and running of Pytokinetics.
This outputted the 89% code coverage and 3 failed tests ====================================================== short test summary info ====================================================== While the code coverage seems good, I am not able to assess if not passing the tests is significant or not. Can the authors address this?
Once these three topics are addressed, I will continue with the remaining check points. ========================Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): |
Review checklist for @the-rccgConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems). import pyrokinetics as pk
pyro = pk.Pyro(gk_file="gs2.input") Considering the stated usecase to standardize input and output files, it feels like one example for each showcasing how to load and save should be the bare minimum. Ideally, some real world example should be done with the resulting object to help people get started with what could be done within the framework. |
We have more "how to" guides written But we can add more example scripts to this and link to the existing examples on the repo! |
Thank you for replying this fast! The How-To's seem to include examples and cover all important use cases I would wish to see. This then seems to simply be an issue of highlighting the fact and removing the duplicate examples heading. Maybe call it "How-To Examples"? While the content is great, the presentation is still lacking. It took two clicks to find an error with wrong indentation (https://pyrokinetics.readthedocs.io/en/latest/howtos/linear_outputs.html). I would recommend going through the How-To's once more and maybe even including them into your test suite to ensure new users will not encounter Problems right away. These How-to's also mix the CLI and Code interface; the previous example only uses the CLI (https://pyrokinetics.readthedocs.io/en/latest/howtos/convert.html). Just a small note that it would be beneficial to probably showcase both methods if both are possible within the code. I'll mark the examples as sufficient, but would really encourage you to go through them once more to remove these inconsistencies. |
@bpatel2107 Last comment on documentation, I do not see any information on contribution guidelines and hence cannot check off the following bullet point. Would be great to see that added!
As an aside on documentation, the ReadMe does define the unit normalization convention readable for plasma physicists. It might be useful to have at least a glossary/explanation of terms for didactic purposes. |
Here the missing namelist are shown. When actually specifying the type of code as follows
|
We decided that a docker was straightforward to add so it is now included and there's an update to the README with instructions on how to use it. |
I would like to thank you for addressing the questions raised, including the docker file. |
@rogeriojorge thank you for your review! |
@the-rccg checking in here - it looks like there have been some updates to address your comments, so please take a look at your earliest convenience. |
thanks for continuing your review @the-rccg , dropping another ping here. |
@kellyrowland I've been able to verify the functionality using version 0.5.0 (as opposed to the current 0.6.1 due to issues with getting pygacode 1.0 running on Windows). It's all working well with that, great work on the code base! I'm sure this will find good use in the community. One thing I was not able to confirm is the Diagnostic usage from the Ideal Ballooning Solver (https://pyrokinetics.readthedocs.io/en/latest/examples/ideal_ballooning.html), which might be due to the older version and the example was added after the release of the 0.5.0 version verified, so ignoring it here. diag = Diagnostics(pyro) # RuntimeError: Diagnostics: Please load gk output files (Pyro.load_gk_output) before using any diagnostic |
thanks very much! @bpatel2107 could you comment on the above? it looks like we have the checkboxes all filled out, but I want to make sure that the |
thanks! I will follow up with my steps here and ping you again in early March. |
Hello, I have returned from leave and am keen to progress with this! Let me know of any additional steps needed. |
@editorialbot check references |
|
@editorialbot generate pdf |
@bpatel2107 it looks like there are several discrepancies between the package and the archive that will need to be aligned:
I think you should be able to make these changes by editing the metadata of the record on Zenodo (see https://help.zenodo.org/faq/#ctechnical-1). |
@kellyrowland Thanks for spotting that, I've made the appropriate changes. I corrected an ORCID and made an institution change for one of our authors so please regenerate the PDF |
@editorialbot generate pdf |
@editorialbot set 10.5281/zenodo.10472477 as archive |
Done! archive is now 10.5281/zenodo.10472477 |
@editorialbot set v0.6.1 as version |
Done! version is now v0.6.1 |
@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#5094, then you can now move forward with accepting the submission by compiling again with the command |
@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 @bpatel2107 on your article's publication in JOSS! Please consider signing up as a reviewer if you haven't already. Many thanks to @the-rccg and @rogeriojorge for reviewing this, and @kellyrowland 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: @bpatel2107 (Bhavin Patel)
Repository: https://github.com/pyro-kinetics/pyrokinetics
Branch with paper.md (empty if default branch): docs/joss_paper
Version: v0.6.1
Editor: @kellyrowland
Reviewers: @the-rccg, @rogeriojorge
Archive: 10.5281/zenodo.10472477
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
@the-rccg & @rogeriojorge, 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 @kellyrowland 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 @rogeriojorge
📝 Checklist for @the-rccg
The text was updated successfully, but these errors were encountered: