-
-
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]: STITCHES: a Python package to amalgamate existing Earth system model output into new scenario realizations #5525
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 |
|
Welcome to the review process 🎉 |
Review checklist for @znichollsConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Just an FYI, I probably won't get to this until the end of the month unfortunately |
Hi there! I'll be taking a look at this very soon, hopefully next week. Thanks again for reaching out to me, @observingClouds. |
Review checklist for @ZeitsperreConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@observingClouds @abigailsnyder My review here is complete. I think that STITCHES does something quite unique that I haven't seen before, and can genuinely see the value of it in performing global-scale impact modelling analyses. Thanks again for asking me to review this software. I think there are quite a few improvements that can be done (many of them are relatively low-effort) that I would gladly open as issues (or submit as fixes in Pull Requests) if they are welcome (and if Pull Requests from reviewers are allowed from reviewers). Please let me know. |
Thanks @Zeitsperre for the update! Please open issues over in the STITCHES' repository. If it helps the discussion of the issue feel free to open a PR as well. |
@observingClouds re conflict of interest: I have published with both Kalyn and Claudia previously. Sorry I should have read that more closely earlier. I would request that the COI is simply noted here (and ideally waived) given that the community is relatively small and I think I can provide an impartial review nonetheless. |
I have also completed my review. In the process, I have made a number of issues (all cross-linked here) to follow up with areas where I haven't ticked the box above yet. In general, I think I could use the package without too much trouble but I would really struggle to extend it beyond its main use case. The major reason is that many of the internal data is built around pandas data frames, but it wasn't clear to me where I should look to understand what sort of form these data frames should take or what the data in them should be. This may require a separate section on the various 'models' used in the code (e.g. https://pyam-iamc.readthedocs.io/en/stable/data.html). Such sections can be a bit painful to write and maintain, but they are generally very helpful for helping new users and maintainers to understand how the system works. In particular, it wasn't clear to me what the recipe should look like nor the archive data (I could sort of guess from the examples, but I am not sure I would guess correctly so explicit docs could be very helpful). The other option would be to use something like pandera to add more structure to the data frames and communicate more explicitly what each column should be and means. Each option comes with pro's and con's, but I think I would find it very hard to build a mental model of the package as it is currently written and documented. Arising from the above, there were a few areas where the functionality of the package wasn't super clear to me. I think adding a section like the 'model' section above and/or refactoring would make it much clearer what is going on. The issues were:
In general, the repo would also benefit from the use of some other tools e.g. code linters and auto-formatters. They would make the code much more readable and introduce very little cost (at least in my opinon). |
(Hi all, I've been meaning to open some issues/PRs in the repo (and will when I find a minute), but I wanted to piggyback on Zeb's great comments here)
I am very much in agreement with this. Having worked with CORDEX/CMIP5/CMIP6 data for many years, I also felt it wasn't entirely clear how I could format my local NetCDF collections for use in
I mentioned in my review that
In my work, I do a fair amount of package maintenance and coding standards enforcement, and would be willing to give a hand in this way. If my schedule allows for it, I'd be more than happy to open a PR. |
Thank you @Zeitsperre and @znicholls for your review and starting the discussion on some action-items. Based on the agreement between your reviews, I suggest @abigailsnyder and their colleagues to already go ahead and address these common issues/suggestions. |
@abigailsnyder please also have a look at some editorial changes at JGCRI/stitches#100. |
@editorialbot generate pdf |
@editorialbot recommend-accept |
|
|
👋 @openjournals/ese-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#5316, then you can now move forward with accepting the submission by compiling again with the command |
@abigailsnyder thank you for integrating the last changes. I have now recommended the manuscript for acceptance. The editor-in-chief will now take over in the next few days, makes a few more checks and then your manuscript should be soon published. In the meantime you can have a look at the final proof above and let us know if it does all look to your satisfaction (or not). |
Ack! I see: "team, T. pandas development." again |
@Zeitsperre Who needs to do something to fix that? |
Oh I see, @Zeitsperre you are a reviewer and @abigailsnyder you are the author. @abigailsnyder can you address this comment in the references? I assume it came up previously too. |
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!
|
Paper:
|
@kthyng Thank you! I finally have more time this morning to proof read and address |
@kthyng I have addressed the raised issues (and one additional, switching from quotations to backtick on line 75 of the PDF). I have done a proof read on my end and will complete another on the proof editorialbot produces here. Thank you! |
@editorialbot generate pdf |
ok ready to go! |
@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 @abigailsnyder! Many thanks to @observingClouds and to reviewers @znicholls and @Zeitsperre 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:
|
Submitting author: @abigailsnyder (Abigail Snyder)
Repository: https://github.com/JGCRI/stitches
Branch with paper.md (empty if default branch): main
Version: v0.13
Editor: @observingClouds
Reviewers: @znicholls, @Zeitsperre
Archive: 10.5281/zenodo.11094934
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
@znicholls & @Zeitsperre, 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 @observingClouds 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 @znicholls
📝 Checklist for @Zeitsperre
The text was updated successfully, but these errors were encountered: