-
-
Notifications
You must be signed in to change notification settings - Fork 36
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]: A Module for Calibrating Impact Functions in the Climate Risk Modeling Platform CLIMADA #6755
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: |
@sunt05, @naingeet thank you very much for agreeing to review this submission. Your expertise is very valuable. With the opening of this issue, the official review process can now start. A good starting point is to create the personal review checklist by commenting Please note that we ask to review the additions made via the PR CLIMADA-project/climada_python#692 as they reflect the new additions and the contents of the submitted manuscript (see this comment). You might want to checkout the particular branch 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, the 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 any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time. Please feel free to ping me (@observingClouds) if you have any questions/concerns. |
@peanutfun thank you for your submission. The review will now start. Please check back regularly and address the reviewers comments and issues as they appear to help us make the review process as interactive and effective as possible. Could you in the meantime try to fix the missing DOIs mentioned above and alternatively provide a URL if a DOI is not available. Thank you! |
Review checklist for @sunt05Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@editorialbot generate pdf |
@editorialbot check references |
|
@observingClouds We were able to replace one work marked with "missing DOIs" with a suitable publication with a DOI. For the rest, we provide URLs:
|
@sunt05 @naingeet We are looking forward to receiving your feedback on the code and the paper draft in CLIMADA-project/climada_python#692. If you have any trouble installing CLIMADA or executing the provided tutorial Jupyter script, please let us know. |
Thanks @peanutfun for the nice work! Could you please address this issue while I was testing your notebook? |
Congratulations to the CLIMADA team on delivering this excellent work, which I believe will be very useful to the climate risk community. After trying out the new development, I have only two minor points for the climada team to consider:
Slightly off-topic, while I understand CLIMADA is a comprehensive codebase with various dependencies, I believe the dependency structure could be simplified for easier installation. Even as an experienced Python user and developer, it took me a while to get it working. New users interested in climate risk analysis rather than Python development might struggle even more. Also, since the complexity largely comes from geospatial analysis packages, I'd suggest focusing on climate risk analysis and leaving the geospatial part to more experienced users. |
@observingClouds Thanks for inviting me to this review. Please see my comments above. Looking forward to future collaborations. |
@sunt05 thank you very much for your comments. Depending on the response of @peanutfun I might ask you one more time to look through any potential edits. |
@peanutfun please have a look at the last comments of sunt05. Please improve the demo notebook accordingly and improve the user experience when installing CLIMADA. I expect the reviews from @naingeet also coming in this week, but please work in the meantime on above's issues to help the interactivity of the review process. Cheers! |
@sunt05 Thank you for your review. I am very glad about your positive feedback. We will add a quickstart section to the tutorial, as you suggested, in the next few days. I understand that the setup for the development version of Climada is quite involved, especially if you have a working setup that does not rely on Conda. Unfortunately, this is a somewhat historical problem we cannot resolve easily. First, we think that geospatial analysis combined with the comparably simple task of impact calculation is the main advantage of Climada, and we would not want to make it optional. Second, the trouble comes from a few C(++)-library dependencies that are used for some submodules of Climada. We plan to move these to the adjacent repository https://github.com/CLIMADA-project/climada_petals, see CLIMADA-project/climada_python#729. Our goal is to make the "Core" https://github.com/CLIMADA-project/climada_python repository and all its dependencies installable only via Pip, which is currently not the case. However, we decided to update these submodules before moving, as they have grown outdated, and are waiting for feedback from collaborators, see CLIMADA-project/climada_python#811. Third, our current module import patterns do not allow well for optional dependencies. Also, testing (combinations of) optional dependencies is a larger task that would overburden our current resources. Finally, we provide a conda-forge package for all Climada releases. We hope this simplifies installation for inexperienced users enough to get started with Climada easily:
Of course, they then cannot switch to a development or feature branch, as you were required to. I hope this clarifies why we currently do not see an easy, quick way to improve the user experience regarding development version installation. We are well aware of the issue, and resolving it is a somewhat long-term goal for us. Still, if you encountered some pitfalls our installation instructions do not cover, we would gladly incorporate any suggestions for improvement. |
@peanutfun thank you for your extended reply. Do I understand it correctly that all dependencies will be easily installable via conda once the branch that is the base of this review will be merged? |
@observingClouds We are merging all our feature branches into an unstable The calibration module under review will become available in the conda-forge package when we create a new release, merging |
Review checklist for @naingeetConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
I finished my review on the code and the paper draft in CLIMADA-project/climada_python#692. Installation proceeded as outlined in advanced instructions for installation and functionally were correctly implemented. The demo notebook code was quite comprehensive and easy to follow. |
@naingeet Thank you for your positive feedback and review! @sunt05 We added a quickstart section and improved the links from the tutorial to the respective Python module documentation sections. We also incorporated your suggestions for fixing typos. See https://climada-python--692.org.readthedocs.build/en/692/tutorial/climada_util_calibrate.html for the latest version of the tutorial. |
Many thanks for addressing my comments @peanutfun! The revised notebook looks brilliant. I'm very happy to suggest the acceptance of your work 🎉 |
Thanks @naingeet and @sunt05 for your time and expertise to review this submission. Your feedback was very valuable. With both of your final positive feedback, I will now work together with @peanutfun on making the submission publication ready. |
Post-Review Checklist for Editor and AuthorsAdditional Author Tasks After Review is Complete
Editor Tasks Prior to Acceptance
|
@editorialbot check references |
|
@peanutfun since the review has now been finished, I will go through the manuscript one more time and would than afterwards ask you to merge the work reviewed here into the main branch so we can make sure that this addition is included in the main software. After this merge has been done, a new software release needs to be made. |
@editorialbot generate pdf |
We updated one reference to a preprint in the manuscript, because the associated publication was released. I'll ask the bot to check references again.
We double checked.
We will merge the reviewed pull request and make a release. This will probably take place next week, and I'll report back afterward.
All Climada releases are automatically released on this Zenodo record: https://zenodo.org/doi/10.5281/zenodo.4598943 The author list is intended to reflect the authors of the entire repository. The JOSS author papers are included in this list. Does that suffice, or should we create a separate Zenodo record for the new release? |
@editorialbot check references |
|
@editorialbot generate pdf |
@observingClouds We released Climada v5.0.0, which includes the reviewed module. This release is available from conda-forge. We also created a Zenodo archive: 10.5281/zenodo.12794729 |
Submitting author: @peanutfun (Lukas Riedel)
Repository: https://github.com/CLIMADA-project/climada_python
Branch with paper.md (empty if default branch): calibrate-impact-functions
Version: v5
Editor: @observingClouds
Reviewers: @sunt05, @naingeet
Archive: Pending
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
@sunt05 & @naingeet, 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 @sunt05
📝 Checklist for @naingeet
The text was updated successfully, but these errors were encountered: