-
Notifications
You must be signed in to change notification settings - Fork 4
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]: Pynamical: Model and Visualize Discrete Nonlinear Dynamical Systems, Chaos, and Fractals #15
Comments
Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @drvinceknight it looks like you're currently assigned as the reviewer for this paper 🎉. ⭐ Important ⭐ If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/jose-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:
|
|
Proof looks good, I will complete the review before the weekend. |
I'm (really) impressed with the software and the paper is well written. A couple of minor things that I have not checked off on my list and would like to run past @gboeing:
The documentation on github is great: I ran everything and it worked exactly as expected. I noted that the documentation rendered by sphinx isn't complete. The autodoc of the api doesn't seem to have picked up the various methods/modules: https://pynamical.readthedocs.io/en/latest/pynamical.html#submodules
The test suite is currently just a test of run to completion of the various functions and not of expected output. I don't think this is something that should hinder publication at all but perhaps @gboeing you would want to think about adding some tests of expected output: For example:
we could at least assert the type of
I didn't see these in the repo but perhaps I missed them? |
@drvinceknight thank you for these thoughtful recommendations. I have made each of these changes you suggested:
|
Thanks for those @gboeing: it all looks great to me know and I gladly recommend this for publication. A very nice library, I might make use of it myself at some point (will let you know if/when I do!). |
@drvinceknight thanks! |
Thanks for your review, @drvinceknight 👏 @sdross0 — Since we have a thorough review of the software side by @drvinceknight, I encourage you to focus your review on the educational value of the package, and especially go over the demos/tutorial and see if you have any recommendations there. As for installation, etc., you could have a student in your group install it, to get a double check and tick off those items on the review checklist. Thank you! |
Pros: Cons: It seems to be designed primarily for 1D maps, that is, 1-dimensional discrete dynamical systems of the form x_k+1 = f(x_k) which isn't explicitly described in the documentation. I think a description of the kinds of models that the software has in mind will make it more successful. It would also help to know how to input your own models into the system. Related to that, I worry the software might not generalize well to the other types of dynamical systems that would appear in an introductory course on dynamical systems (say, following Strogatz book), mostly because of the restriction to 1D maps. Most such courses do include 1D maps at the beginning of the course, and perhaps that is where Pynamical could be most useful. |
@gboeing See how you can address the comments above in the documentation, examples and paper, and let us know. You may want to add a note about the current limitations of Pynamical, whether and how it could be expanded beyond those limitations, and how it might be used in a standard course in dynamical systems. |
@sdross0 thank you for these thoughtful recommendations. I have made each of these changes you suggested:
|
@drvinceknight, @sdross0 — Thank you both for reviewing this submission to JOSE. Since you both have ticked all items in your checklist, and made constructive comments that were addressed, all I need now is a confirmation on a comment below that you are ready to recommend acceptance. This will be the first paper in JOSE and we're excited and grateful to you for being a part of this plunge into a new model of scholarly publication! |
I confirm 👍
Awesome: 👍 |
I recommend acceptance. |
@gboeing The next step is for you to make an archive on a service like Zenodo, get a DOI, and post the DOI here. (You may want to tag a release on your GitHub repo right before archiving.) |
@labarba 10.5281/zenodo.1294299 |
@whedon set 10.5281/zenodo.1294299 as archive |
OK. 10.5281/zenodo.1294299 is the archive. |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippet:
This is how it will look in your documentation: We need your help! Journal of Open Source Education 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:
|
@gboeing -- your paper is now published in JOSE @drvinceknight, @sdross0 — Thank you again for your review 🙏 |
Submitting author: @gboeing (Geoff Boeing)
Repository: https://github.com/gboeing/pynamical
Version: 0.1.2
Editor: @labarba
Reviewer: @drvinceknight, @sdross0
Archive: 10.5281/zenodo.1294299
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) 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
@drvinceknight, @sdross0, 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://jose.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @labarba know.
Review checklist for @drvinceknight
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @sdross0
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?The text was updated successfully, but these errors were encountered: