-
-
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]: NCDatasets.jl: a Julia package for manipulating netCDF data sets #6504
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: |
Review checklist for @boriskausConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
The editorialbot did not detect the MIT license of the package. I just changed the markdown license to a plain-text license which seems to be more in line with recently published joss paper. |
@Alexander-Barth: I was checking your readme, and while it all works, I think it could be improved by changing the order of the sections in README by first creating a netCDF file |
Thanks @boriskaus for this good suggestion! I implemented this change here: Alexander-Barth/NCDatasets.jl@2fe4efb . |
@Alexander-Barth as part of the review they ask me to verify that your performance claims are correct. You did a nice job of describing how to set this up and run it, but unfortunately I don’t have access to a Linux machine where I have root access. The testing doesn’t seem to work on a Mac. |
In fact, the performance timing are quite similar if I disable the OS-specific flushing of the memory:
The standard deviation of the timings is a bit larger but mean/minimum/mean are similar. Would it be OK, if I implement an option to the script like |
Yes that seems a fine idea to me |
I updated the benchmark here: Alexander-Barth/NCDatasets.jl@27a2992 In fact, "not dropping the caches" is now the default so that you should be able to reproduce the benchmark simply with running the script https://github.com/Alexander-Barth/NCDatasets.jl/blob/master/test/perf/README.md |
thanks a lot. I managed to reproduce this on Mac (M2). The numbers are fairly similar and Julia remains on top:
|
Thank you very much @boriskaus ! |
@majensen I will try to submit my review next week. Sorry for the delay. |
Review checklist for @lanariConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@Alexander-Barth, please accept my apologies for the delay in reviewing your work. @majensen This is a well-designed package and I think it will be very useful for the community. The repository contains all the necessary files and the examples given are easy to reproduce. The software paper is clear and well written. I strongly recommend publication. For comparison, here is the package's performance on a MacBook Pro 2.3GHz 8-core Intel Core i9:
|
@lanari : There is no problem at all. Thank you very much for your time looking at my package! |
Ok @Alexander-Barth - I will have an editorial look at the paper itself, this usually leads to very minor changes. Can you create an archive of the code base using Zenodo or similar? I have a little tutorial here. Then report the version and the DOI back to this thread. Thanks! |
I just created a new release with the Zenodo-Github link activated: The DOI is 10.5281/zenodo.11067062 corresponding to the NCDatasets version v0.14.4. |
@editorialbot set 10.5281/zenodo.11067062 as archive |
Done! archive is now 10.5281/zenodo.11067062 |
@editorialbot set v0.14.4 as version |
Done! version is now v0.14.4 |
👋 @majensen - it looks like this one is almost ready to accept. Could you follow up in this thread? Thanks! |
Hi! I'll take over now as Track Associate Editor in Chief to do some final submission editing checks. After these checks are complete, I will publish your submission!
|
@editorialbot generate pdf |
@Alexander-Barth Can you update your Zenodo title and author list to exactly match your JOSS paper? This is a preference not a requirement so the JOSS archive is self-consistent. |
@Alexander-Barth Also please check the capitalization in your references. You can preserve capitalization by placing {} around characters/words in your .bib file. |
Thanks a lot for spotting these issues! I fixed the capitalization in the references, and updated author/title of the Zenodo archive to match the JOSS paper. While proof-reading the paper, I made these (small) changes to the paper: Alexander-Barth/NCDatasets.jl@a007ff1 Alexander-Barth/NCDatasets.jl@f8d7643 I hope that this is ok for you :-) (I do not have any other changes) |
@editorialbot generate pdf |
@Alexander-Barth Yes, looks great! |
@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 on your new publication @Alexander-Barth! Many thanks to @majensen and to reviewers @lanari and @boriskaus for your time, hard work, and expertise!! JOSS wouldn't be able to function nor succeed without your efforts. |
🎉🎉🎉 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:
|
@Alexander-Barth If you're interested in joining JOSS as a reviewer, please sign up in the database here: https://reviewers.joss.theoj.org/. |
Thanks @kthyng , @majensen, @boriskaus and @lanari for your work as editor and reviewer! I would be glad to help as a reviewer too in future :-) |
Submitting author: @Alexander-Barth (Alexander Barth)
Repository: https://github.com/Alexander-Barth/NCDatasets.jl
Branch with paper.md (empty if default branch): joss
Version: v0.14.4
Editor: @majensen
Reviewers: @lanari, @boriskaus
Archive: 10.5281/zenodo.11067062
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
@lanari & @boriskaus, 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 @majensen 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 @boriskaus
📝 Checklist for @lanari
The text was updated successfully, but these errors were encountered: