Skip to content
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]: DynamicalBilliards.jl : An easy-to-use, modular and extendable Julia package for Dynamical Billiard systems in two dimensions. #458

Closed
18 tasks done
whedon opened this issue Nov 15, 2017 · 69 comments
Assignees
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review

Comments

@whedon
Copy link

whedon commented Nov 15, 2017

Submitting author: @Datseris (George Datserus)
Repository: https://github.com/JuliaDynamics/DynamicalBilliards.jl
Version: v1.6.1
Editor: @kyleniemeyer
Reviewer: @ahwillia
Archive: 10.5281/zenodo.1059092

Status

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/753469f6b18c9c38127a7727d13c87cd"><img src="http://joss.theoj.org/papers/753469f6b18c9c38127a7727d13c87cd/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/753469f6b18c9c38127a7727d13c87cd/status.svg)](http://joss.theoj.org/papers/753469f6b18c9c38127a7727d13c87cd)

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 questions

@ahwillia, please carry out your review in this issue by updating the checklist below (please make sure you're logged in to GitHub). The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @kyleniemeyer know.

Conflict of interest

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Version: Does the release version given match the GitHub release (v1.6.1)?
  • Authorship: Has the submitting author (@Datseris) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
@whedon
Copy link
Author

whedon commented Nov 15, 2017

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @ahwillia 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/joss-reviews) repository. As as 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:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

@kyleniemeyer
Copy link

👋 @Datseris @ahwillia @mlxd

@Datseris
Copy link

Thank you very much for accepting to review my submission.

@mlxd
Copy link

mlxd commented Nov 15, 2017

Happy to participate. It will likely be the coming weekend before I can spend time going through everything in detail. As suggested by @kyleniemeyer I will leave comments to the above check-box items.

@ahwillia
Copy link

Overall this looks excellent - really appreciate your attention to detail while putting this together!

@Datseris - what do you think about having an examples/ folder containing jupyter notebooks? These could just contain the code that you have under Tutorials in the docs (but without the long text). That way people can flip back and forth between reading your docs and trying it. Right now there is an awkward copy-paste step.

Other than that I'm inclined to approve this. But we can wait for @mlxd to weigh in.

@ahwillia
Copy link

ahwillia commented Nov 15, 2017

Oh two other short comments

Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Is this listed anywhere explicitly?

References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Do you mind adding DOI links to the references in your paper.md document? Thanks!

@Datseris
Copy link

Thanks for the kind words @ahwillia . I've just added guidelines for issue reports.

I will now do your other requests:

  • Add DOIS to paper.md
  • Add a dedicated paragraph on the homepage of the documentation about support (we have our own chatroom on gitter)
  • Add a dedicated contributors_guide file either in the repo first page or in the documentation page.

About the Jupyter notebooks... Is it possible to avoid this step? My only issue is that I have never used Jupyter and it make take a while/be tricky to set it up etc.

I know it is kind of annoying to copy/paste that much but fortunately the documentation allows you to copy an entire code-block by clicking the top - right corner of each code block.

@ahwillia
Copy link

ahwillia commented Nov 15, 2017 via email

@Datseris
Copy link

@ahwillia I have done the requested changes. Now all citations of the paper.md have DOIs, and the homepage has 2 sections: One for support and one for contributing.

See this link: https://juliadynamics.github.io/DynamicalBilliards.jl/latest/#support

Of course, the changes apply only on the latest version, and not on the release tag 1.6.1. Should a new tag be required let me know and I will tag it.

@ahwillia
Copy link

I'm happy to accept this - I was able to install, run tests, and reproduce some simple plots. The package supports a well-defined need and is reasonably mature. Looking forward to seeing future developments!

Now we'll just wait on @mlxd to check it over.

@mlxd
Copy link

mlxd commented Nov 18, 2017

Everything is clearly documented, installation and use is as described. This is a very useful package, and I'd be happy to accept this too. I agree with all of @ahwillia's comments and checkboxes.

I do have one suggestion on the documentation for the examples:
For the Julia-logo Billiard example (and potentially elsewhere) [https://juliadynamics.github.io/DynamicalBilliards.jl/stable/tutorials/examples/], the file save path seems Windows specific (sname = "C:\***\anim").

While it works for me on Mac, it does create a lot of "C:***\anim_99.png" files in the current directory, which may go elsewhere on a Windows machine due to the path (I have not tested). I would suggest a more platform-independent save string with the correct format for Win/Mac/LNX. Maybe explicitly setting the savedir as the current working directory (pwd()) would suffice for all platforms, and users can run the example code without changing. Though, feel free to ignore this, as it is merely a suggestion.

@Datseris
Copy link

This was simply a "dummy" name (indicated by the ***)... I expected users to put as a string wherever they want to save their files.

I will just replace pwd() since you are right, it is better.

@mlxd
Copy link

mlxd commented Nov 18, 2017

Great. Excellent work on this. It was very easy to review and recommend for acceptance. Unless @ahwillia has anything else, I think we are done.

@Datseris
Copy link

Awesome. Thank you for the swift and concise review!

@kyleniemeyer
Copy link

@Datseris sounds like we are nearly good to go—let me know when you make the change mentioned by @mlxd above.

@Datseris
Copy link

@kyleniemeyer
Copy link

@Datseris ok, there are a few changes required for the paper.

  1. The references need to be extracted from the Markdown paper file, put in a BibTeX .bib file, and then cited in the paper. You can see an example in the JOSS author guidelines. This is necessary for our LaTeX-based compiling of the paper PDF, and also depositing the citation info in CrossRef.

  2. The animated GIF you have is nice for the README, but it won't work for the paper, which is generated as a static PDF. Instead of the logo, could you perhaps include an example figure using results produced by the package (perhaps from one of the examples/tutorials), along with a brief description? That would be more useful and informative.

  3. As for the actual content of the paper, the layout you have is not quite appropriate—again, it works for a README-type document, but not the paper. Links to your software repository will already be included in the paper (see an example compiled PDF for a different paper under review), so please remove those from the top.

Also, to the summary or introductory paragraph, could you describe the research application of the software for a diverse, non-specialist audience? Many people may not be familiar with billiard systems in the context of research.

  1. A few minor typos/suggested edits:
  • "In [2] this is described perfectly, where Bunimovich and Vela-Arevalo (two pioneers in the field) summarize new insights in the field of dynamical billiards" -> "Bunimovich and Vela-Arevalo (two pioneers in the field) summarized new insights in the field of dynamical billiards [2]."
  • "Our package has plenty of applications" is quite informal and not descriptive
  • "which results to accuracies" -> "which results in accuracies"
  • I would eliminate the use of italics to emphasize words throughout; it makes the paper read as more persuasive than is intended/appropriate, I think.

@Datseris
Copy link

@kyleniemeyer I have done all the changes you requested. Please see the latest files here: https://github.com/JuliaDynamics/DynamicalBilliards.jl/tree/master/paper

I have a problem though. You said that "links to the repository will be included in the paper". I saw the example you cited but nowhere in that example there was a link to a documentation page. There was a link to a GitHub repository, but for me this is not enough.

I would like to have a direct link to the documentation of the package somewhere in the paper. This could be either at the side, as part of this typical information or in-text. I do not mind.

The users of a package are users, not developers. They should never have to know what git or GitHub is. To use the package you should just have a link to the documentation without having to go through links to other links to search for a documentation page.

@kyleniemeyer
Copy link

@Datseris sure, that's fine—perhaps you could link on "detailed documentation archive" in the summary at the top of the paper?

@Datseris
Copy link

@kyleniemeyer I am glad we could come to an agreement. I have added an in-text hyperlink.

Is that all from my side?

@kyleniemeyer
Copy link

@Datseris thanks! I'm going to see if the paper compiles. The last step from you is archiving the final version of the software etc. (like using Zenodo) and then telling me the DOI here; but, wait until we're sure the paper doesn't need any more changes.

@kyleniemeyer
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Nov 19, 2017

Attempting PDF compilation. Reticulating splines etc...

@kyleniemeyer
Copy link

@arfon ruh roh. any way to see what went wrong?

@whedon
Copy link
Author

whedon commented Nov 20, 2017

https://github.com/openjournals/joss-papers/blob/joss.00458/10.21105.joss.00458.pdf

@arfon
Copy link
Member

arfon commented Nov 20, 2017

@kyleniemeyer @Datseris - the links in the paper are looking a bit better now.

@kyleniemeyer
Copy link

Yes, looks great! Ready to accept/publish.

@arfon
Copy link
Member

arfon commented Nov 20, 2017

@ahwillia - many thanks for your review here and to @kyleniemeyer for editing this submission ✨

@Datseris - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00458 ⚡️ 🚀 💥

@arfon arfon closed this as completed Nov 20, 2017
@Datseris
Copy link

Awesome! Thanks everybody for their effort!

@Datseris
Copy link

@ahwillia @kyleniemeyer I am sorry about this but is it possible to recompile the paper? Or "update" it?

I realized that even though I am acknowledging people properly in the repository page, this is not very transparent in the paper. Therefore I copied the Acknowledgements page from the repository and added it to the paper.md as well, so that they are more transparent.

I am sorry for forgetting this.

@arfon
Copy link
Member

arfon commented Nov 20, 2017

@Datseris - yes, that's OK. The @username syntax in the paper is confusing Pandoc though:

screen shot 2017-11-20 at 6 48 23 am

@arfon
Copy link
Member

arfon commented Nov 20, 2017

I think you should be able to quote their usernames in back-ticks e.g.

`@arfon`

@Datseris
Copy link

Datseris commented Nov 20, 2017

@arfon Thanks a lot for the flexibility you provide. I changed the nametags to have back-ticks.

@Datseris
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Nov 20, 2017

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Nov 20, 2017

https://github.com/openjournals/joss-papers/blob/joss.00458/joss.00458/10.21105.joss.00458.pdf

@Datseris
Copy link

@arfon everything is fine with the pdf! The only issue is that when I go to the DOI page,
the pdf linked there is not updated (it still has the old version): http://joss.theoj.org/papers/10.21105/joss.00458

@arfon
Copy link
Member

arfon commented Nov 20, 2017

@arfon everything is fine with the pdf! The only issue is that when I go to the DOI page,
the pdf linked there is not updated (it still has the old version): http://joss.theoj.org/papers/10.21105/joss.00458

Yep, I've not updated them yet. Will get to this either later today or tomorrow.

@arfon
Copy link
Member

arfon commented Nov 21, 2017

@arfon everything is fine with the pdf! The only issue is that when I go to the DOI page,
the pdf linked there is not updated (it still has the old version): http://joss.theoj.org/papers/10.21105/joss.00458

@Datseris - ok, these should be updated now.

@Datseris
Copy link

@arfon thank you very much, everything looks very nice. Quick question: what is the "formal"/official form for the bibtex entry for your journal? From my DOI link I can see:

Citation:
Datseris, (2017). DynamicalBilliards.jl: An easy-to-use, modular and extendable Julia package for Dynamical Billiard systems in two dimensions.. Journal of Open Source Software, 2(19), 458, doi:10.21105/joss.00458

Is there any way to export the citation as a bibtex entry?

@kyleniemeyer
Copy link

@Datseris you can try using https://www.doi2bib.org/; we used to rely on it, but it isn’t always working.

@danielskatz
Copy link

also see openjournals/joss#341

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review
Projects
None yet
Development

No branches or pull requests

7 participants