-
-
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]: Empirical: A scientific software library for research, education, and public engagement #6617
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:
|
|
Software report:
Commit count by author:
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
License info: 🟡 License found: |
@LTLA and @bramvandijk88 - Thank you for agreeing to review this submission. This is the review thread for the paper. All of our communications will happen here from now on. As mentioned above, you can use the command There are also links to the JOSS reviewer guidelines (https://joss.readthedocs.io/en/latest/reviewer_guidelines.html) The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention We aim for reviews to be completed within about 2-4 weeks. Please let me know if you require additional time. We can also use Please feel free to ping me (@mahfuz05062) if you have any questions/concerns. |
Review checklist for @bramvandijk88Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Heya, I've started my review in between some teaching obligations. I generally dislike debugging paths and compiling C code, so I'll leave the problem-solving up to the authors: In following the installation instructions on Mac OSX (Ventura 13.2.1) I get the following missing file:
|
Review checklist for @LTLAConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Alright, apologies for the delay. Here's my review: This is a very interesting library. I have long wondered how to manipulate HTML from within C++, and Empirical provides an ergonomic way of doing so. It offers the possibility of writing web applications almost completely in C++, without any need to touch Javascript or associated frameworks. I can forsee this being useful for scientific programmers who want to build a simple application based on C++ tools without requiring any web development knowledge. In this respect, Empirical targets a similar market to Shiny (R) or dash (Python), but for C++ developers. That said, more experienced web developers will probably not use these features, as direct DOM manipulation is discouraged in frameworks like React. The extra debugging information for the standard data structures is nice, especially for WebAssembly where debugging segfaults is even more tedious than usual. Perhaps the authors might consider checking whether a macro could be used to seamlessly switch between The paper itself is well-written and concise. Possibly too concise, actually: it might have benefited from one or two short code examples to demonstrate the ease-of-use of the UI manipulation, bounds-checking, etc. Then readers can get a better grasp of what the library actually does, before deciding whether to proceed to the documentation itself. On some other minor points:
Related issues: |
Thanks for bringing this to our attention! We've updated the documentation to clarify a missing step of initializing the project subrepositories. If you do
|
Installation was running for ~ 10 minutes until I ran into this:
|
On a different note (I'm sure the installation will eventually work out), I'm happy @LTLA went into depth on the C stuff (e.g. the smart pointer comments), as it's been over 6 years since I did anything with C. That leaves room for me to test the examples thoroughly, which I am quite fond of doing. I hope that together with @LTLA's comments, we'll cover all the bases. |
Hi @bramvandijk88 --- Apologies for the friction here!! Fortunately, I think we just fixed this issue as @mercere99 just ran into it last week. (This PR devosoft/Empirical#515). The fix has been merged onto the Looking forward to getting your feedback on the examples, and please do let us know of any further issues you may run into. |
Hi! I got through the installation now, and have completed the checklist. I have also tested some of the (basic) examples and tested some self-written code using WASM myself. Here's my review: The authors clearly did all the due diligence when it comes to installation and automated testing. Barring a few minor hick-ups (which they resolved), everything installed smoothly and I got into the actual fun part. Similar to the other reviewer, I feel like the paper could have provided a bit more details. If anything, the current paper undersells the utility of this tool, which is definitely a wheel I am not going to reinvent in the future (in fact, I tagged some students to pick some of this up and build something cool with it). I've always enjoyed explorable and sharable simulations, which is why I gradually moved from C/C++ to javascript/typescript. While I was made aware of Webassembly, getting all of this properly set up sounded like too much of a hassle. TLDR: the authors did the community a great service! I would say my only (minor) disappointment (which may be a misconception) is that running the code in a web browser still requires a server, meaning it will not be as easy for students that are just learning to code: e.g. they first need to learn how to install python and launch a local server using that. It's a minor annoyance I have when I am faced with 50 students all of which have their own errors in "trying to install stuff", but I suppose that's not always avoidable. It would be amazing if the authors considered to, in the future, porting their tool to something like WASMfiddle which would further enable educators such as myself to use their software in teaching students basic programming skills. On the actual structures provided by Empirical, I did not have the time to test all the individual types (worlds, genomes) and their accompanying methods (GetNumOrgs, etc.). However, the provided examples in the "Built with Emperical Gallery" (all of which have a link to their original code) would give a starting coder plenty of room to work with. I am a little surprised by the performance of e.g. the cancer model still appearing quite slow, but perhaps there is a lot more going on under the hood than I realise. None of this invalidates the software anyway. In other words, it seems like any enthusiast would get something running with relatively little effort. Congratulations to the authors for this amazing piece of software! |
@bramvandijk88 Thank you for completing the review! I see the other review is also going very well and I will check back later. |
@LTLA Please let us know if you have any questions or need any assistance on the remaining task of the checklist. |
Sorry for the late reply; lgtm. Some of my issues are still open but are trending in the right direction so not a blocker. |
Pinging my co-maintainers and try to get a review on the de-jquerifyication PR that's open. Hopefully progress on that soon. |
|
👋 @openjournals/csism-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#5432, then you can now move forward with accepting the submission by compiling again with the command |
@mmore500 - As track editor, I've proofread the paper, which looks good, except for some minor issues in the references. Please merge devosoft/Empirical#520 or let me know what you disagree with. |
@mmore500 - Also, I see the metadata in the zenodo repository doesn't match the paper, which we ask to happen. I see some discussion about this in #6617 (comment). How should we resolve this? |
Merged 520, thanks for those. |
@editorialbot recommend-accept proofing again to make sure everything worked... |
|
What is the nature of the mismatch in the Zenodo repository? I think the titles should exactly match now. Is it the author list? |
👋 @openjournals/csism-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#5434, then you can now move forward with accepting the submission by compiling again with the command |
yes. |
|
I will release to zenodo with an udpated author list. Should take about 10 minutes I expect. |
you should just be able to update the metadata - you don't need to make a new release or deposit Also, I probably won't get back to this for 6-24 hours |
There seems to be some issue with permissions in our organization on Zenodo that's not letting me update the metadata. (Or I'm just not using the interface correctly.) |
The new version is v1.1.5 and the new Zenodo DOI is https://doi.org/10.5281/zenodo.11420797 |
@editorialbot set v1.1.5 as version |
Done! version is now v1.1.5 |
@editorialbot set 10.5281/zenodo.11420797 as archive |
Done! archive is now 10.5281/zenodo.11420797 |
@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 to @mmore500 (Matthew Andres Moreno) and co-authors on your publication!! And thanks to @LTLA and @bramvandijk88 for reviewing, and to @mahfuz05062 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: @mmore500 (Matthew Andres Moreno)
Repository: https://github.com/devosoft/Empirical/
Branch with paper.md (empty if default branch): joss-paper
Version: v1.1.5
Editor: @mahfuz05062
Reviewers: @LTLA, @bramvandijk88
Archive: 10.5281/zenodo.11420797
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
@LTLA & @bramvandijk88, 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 @mahfuz05062 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 @bramvandijk88
📝 Checklist for @LTLA
The text was updated successfully, but these errors were encountered: