Closed editorialbot closed 1 month ago
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:
@editorialbot commands
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@editorialbot generate pdf
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1017/9781108601269 is OK
- 10.21105/jose.00100 is OK
- 10.1017/9781107588783 is OK
- 10.5194/gmd-9-1937-2016 is OK
MISSING DOIs
- No DOI given, and none found for title: XClim Official Documentation
INVALID DOIs
- None
Software report:
github.com/AlDanial/cloc v 1.90 T=0.42 s (576.3 files/s, 421510.0 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
HTML 41 6668 288 27461
Jupyter Notebook 83 0 88840 17821
JavaScript 21 4573 4489 16561
CSS 12 811 115 2904
PO File 45 1007 0 2284
YAML 6 8 6 1203
TeX 2 32 0 494
Markdown 25 316 0 447
Python 2 119 281 250
SVG 5 0 1 29
-------------------------------------------------------------------------------
SUM: 242 13534 94020 69454
-------------------------------------------------------------------------------
Commit count by author:
156 mike-morris-codes
15 Michale Morris
Paper file info:
📄 Wordcount for paper.md
is 1069
✅ The paper includes a Statement of need
section
License info:
🔴 License found: Creative Commons Attribution Share Alike 4.0 International
(Not OSI approved)
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
👋 @aulemahal @brian-rose we will conduct the review in this issue.
Please read through the above information and let me know if you have any questions about the review process.
Thank you 🙏
paper.md
file include a list of authors with their affiliations?@editorialbot commands
Hello @acocac, here are the things you can ask me to do:
# List all available commands
@editorialbot commands
# Add to this issue's reviewers list
@editorialbot add @username as reviewer
# Remove from this issue's reviewers list
@editorialbot remove @username from reviewers
# Get a list of all editors's GitHub handles
@editorialbot list editors
# Assign a user as the editor of this submission
@editorialbot assign @username as editor
# Remove the editor assigned to this submission
@editorialbot remove editor
# Remind an author, a reviewer or the editor to return to a review after a
# certain period of time (supported units days and weeks)
@editorialbot remind @reviewer in 2 weeks
# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist
# Set a value for version
@editorialbot set v1.0.0 as version
# Set a value for branch
@editorialbot set jose-paper as branch
# Set a value for repository
@editorialbot set https://github.com/organization/repo as repository
# Set a value for the archive DOI
@editorialbot set 10.5281/zenodo.6861996 as archive
# Mention the EiCs for the correct track
@editorialbot ping track-eic
# Run checks and provide information on the repository and the paper file
@editorialbot check repository
# Check the references of the paper for missing DOIs
@editorialbot check references
# Generates the pdf paper
@editorialbot generate pdf
# Recommends the submission for acceptance
@editorialbot recommend-accept
# Generates a LaTeX preprint file
@editorialbot generate preprint
# Flag submission with questionable scope
@editorialbot query scope
# Get a link to the complete list of reviewers
@editorialbot list reviewers
# Creates a post-review checklist with editor and authors tasks
@editorialbot create post-review checklist
# Open the review issue
@editorialbot start review
@editorialbot remind @brian-rose in 1 day
Reminder set for @brian-rose in 1 day
Hi @aulemahal, thanks for going through the review check list. May I ask if you can expand further (add comments) of the unchecked items, particularly in the pedagogy section? This info might be helpful for the authors to implement changes if necessary. Thank you.
Hi! My review is still in progress. I have gone through the 3 first chapters of the document, I plan to finish the review this week. Sorry for the delay!
Hi! My review is still in progress. I have gone through the 3 first chapters of the document, I plan to finish the review this week. Sorry for the delay!
Thanks for the update 🙏
:wave: @brian-rose, please update us on how your review is going (this is an automated reminder).
paper.md
file include a list of authors with their affiliations?A couple of quick issues I noted in a first pass, before taking a deeper dive into the content:
I have finished going through the guidebook!
From the review checklist, the only remaining "issue" is indeed that there seems to be no versioning on the guidebook repo. Otherwise, I find this document to be quite interesting. It is well written and complete. It is long, but (almost) no parts seemed superfluous. The topic is complex and to learn both the theory and the practical stuff, a user needs to take their time going though the book.
The following comments are not really publication-blocking to me.
The only section I found tedious was the data download. Station stuff is concise enough with ec3
and the usage of siphon
on PAVICS is clear too. However, all those custom functions with request
for the other dataset make the code look overly complex (too me). I would suggest the following ideas to make this part less cumbersome:
intake-esm
with Google's CMIP6 catalog. See this docwget
scripts (or even globus).All my other comments are on details of the python code. Should I put them elsewhere ? As issues in the original repo ?
pandas
. Xarray and Pandas have very similar but different APIs, as to not confuse the readers, I would suggest showing early in the document how to transform DataFrames
into Datasets
cleanly and only computing and plotting with xarray from then on.dask.array.apply_along_axis
is used in many places. I would suggest using xr.apply_ufunc
instead (with vectorize=True
). Also, the first place it is used (section 3.2), the data isn't even in a dask array.xr.open_dataset
. PAVICS suggests some chunking for its principal datasets and for others, chunks='auto'
might be ok for what this guidebook tries to show. dask.config.set(num_workers=XX)
might save some headaches and machine jamming. A bonus section or links to tutorials about the distributed Client
(and its dashboard!) might be useful.xclim.ensembles.create_ensemble
can merge datasets along dimensions like time
, the input arguments simply need to be given appropriately.xclim.atmos
instead of xclim.indices
.Small grammatical fix in the JOSE paper text: in the Guidebook Content section, line 51 of the rendered PDF. Replace
Chapter 2 explains how climate change projections are made and what their limitations.
with
Chapter 2 explains how climate change projections are made and what their limitations are.
I have finished going through the entire guidebook.
Overall I found the material to be well-organized and readable. The scope seems reasonable relative to the stated needs and audience. It's not a short read! But seems like a valuable teaching resource. Nice work!
Although complete reproducibility of the code examples doesn't seem to be in the list of review requirements, I thought it would be useful to take this opportunity to run through all the content and flag anything that needs updating. I opened a few issues (linked above) describing the problems I encountered. At least one of these issues (with the PCIC data access) was already flagged by @aulemahal. The issue with unit errors in xlim is causing failures in several notebooks.
I tend to agree with @aulemahal about the rather cumbersome data downloads. There is an actively maintained tutorial on accessing CMIP6 data on Google cloud with intake-esm in the Project Pythia CMIP6 Cookbook, perhaps this is helpful.
In terms of checklist items, I left unchecked "Version", "Installation Instructions" and "References" for reasons described above.
Thanks @brian-rose and @aulemahal for your helpful feedback! Apologies for all of the Python bugs, we've had quite a few users run the code without error so I wonder if there are package version issues that need to be sorted - I'll look into it. @acocac how do we proceed from here? I'm not used to this sort of open review process, so do I start making the revisions right away or will you write an editor's decision like a traditional journal? I'll wait to hear from you before I proceed.
Thanks @brian-rose and @aulemahal for your helpful feedback! Apologies for all of the Python bugs, we've had quite a few users run the code without error so I wonder if there are package version issues that need to be sorted - I'll look into it. @acocac how do we proceed from here? I'm not used to this sort of open review process, so do I start making the revisions right away or will you write an editor's decision like a traditional journal? I'll wait to hear from you before I proceed.
Hi @mikemorris12, following the review process in the Submitting a paper to JOSE section, you should:
intake-esm
). Please refer to semantic versioning 2.0.0 for further info. Hi @acocac, I think I've addressed each of the reviewer’s comments. Here is a line-by-line response:
For @aulemahal ‘s comments:
esgf-pyclient
and Google Cloud data access tutorials, I’ve added a third one (Section 3.6) which shows users how to use intake-esm
to access data. I’ve also mentioned in the preamble of Section 3.4 (the esgf-pyclient
and model analysis tutorial) that these other options exist and are easier to use.xarray
is the standard (and superior) package for working with climate data. However, many new users are experienced working with tabular data but have no experience withxarray
, which is why we started the tutorial using pandas to manipulate the station data.dask.array.apply_along_axis
have been replaced with xr.apply_ufunc
.chunks=’auto’
allows the user to load larger amounts of data from PAVICS via OpenDAP.xarray
and dask
documentation on parallelization.xclim.ensembles.create_ensemble
to merge two datasets along the time
dimension, but I couldn’t get it to work properly. The resulting dataset had the wrong time values for the latter half of the record. For now, I’ve mentioned that it’s possible, but I can’t add instructions on how to do it since I couldn’t get it to work.xclim.indices
have been switched to xclim.indicators.atmos
. For @brian-rose ‘s comments:
xclim
citation in the manuscript.intake-esm
.A question for @acocac is when exactly I should merge the changes into master, create the new release, and deposit the repo in Zenodo. You said, “tag me when you complete the above steps”, but you also said that branch merge and the Zenodo deposit should take place after successful completion of the review. I assume that the reviewers need to revisit their checklists before the review is considered complete. Please advise on how to proceed.
A question for @acocac is when exactly I should merge the changes into master, create the new release, and deposit the repo in Zenodo. You said, “tag me when you complete the above steps”, but you also said that branch merge and the Zenodo deposit should take place after successful completion of the review. I assume that the reviewers need to revisit their checklists before the review is considered complete. Please advise on how to proceed.
@mikemorris12 I'll ask the reviewers to confirm they are satisfied and recommend publication. Then, you should complete the final steps incl. branch merge.
@brian-rose @aulemahal - can you confirm if the above comments/implementations by the author are ok for you? Would you recommend the work for publication?
Thank you!
@editorialbot generate pdf
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@acocac I confirm that I am satisfied with the author's revisions, and I recommend this work for publication in JOSE!
@acocac I also confirm that I am satisfied. And I recommend this work for publication in JOSE.
@mikemorris12 congrats your work has been recommended for publication - The next step is to merge the branch and created a tagged release on Github. You must also archive on either https://zenodo.org/ or https://figshare.com/ and obtain a DOI (which you'll need to give to us). I can then move to the final acceptance and publication steps. I'm adding a post-review checklist which could be useful to track the progress on your publication.
@editorialbot set <DOI here> as archive
@editorialbot set <version here> as version
@editorialbot generate pdf
@editorialbot check references
and ask author(s) to update as needed@editorialbot recommend-accept
Hi @acocac, I've completed the steps of the checklist. Here is the Zenodo DOI: https://doi.org/10.5281/zenodo.12636292. Thanks for a great review experience, and thank you to @brian-rose and @aulemahal for your helpful feedback and positive reviews! I'm very pleased that this work will be published in the JOSE.
@mikemorris12 thanks for depositing the software in Zenodo. Before proceeding with the editor steps, can you please ensure it has the same license of the software, in this case Creative Commons Attribution Share Alike 4.0 International?
Please note you can use the Zenodo/GitHub integration for making easier the step of depositing future releases of your software.
@editorialbot set master as branch
Done! branch is now master
@editorialbot check repository
Software report:
github.com/AlDanial/cloc v 1.90 T=0.54 s (507.5 files/s, 462966.1 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
HTML 43 8097 481 35743
Jupyter Notebook 109 0 146929 23686
JavaScript 23 4591 4515 16653
CSS 14 809 118 2906
PO File 45 1007 0 2284
TeX 2 33 0 510
Markdown 25 315 0 447
Python 2 119 281 250
YAML 5 8 6 148
SVG 6 0 1 30
-------------------------------------------------------------------------------
SUM: 274 14979 152331 82657
-------------------------------------------------------------------------------
Commit count by author:
167 mike-morris-codes
16 Michale Morris
@editorialbot generate pdf
Paper file info:
📄 Wordcount for paper.md
is 1070
✅ The paper includes a Statement of need
section
License info:
🔴 License found: Creative Commons Attribution Share Alike 4.0 International
(Not OSI approved)
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
HI @acocac, apologies for using the wrong license on the Zenodo archive - it was previously the Creative Commons Attribution 4.0 International license, whereas the GitHub repo used the Creative Commons Attribution Share Alike 4.0 International license. They now both use the Creative Commons Attribution Share Alike 4.0 International license.
Some inconsistencies remain in the license descriptions. The README states CC-BY-SA but it shows the CC-BY badge and link below. Also, you have code as part of the Guide, which should be licensed separately under an OSI-approved license (for example, BSD-3 or MIT for minimum restrictions).
By the way, what is the reason for the change from CC-BY to CC-BY-SA? By adding a copyleft condition, you do restrict some uses, so be sure you have good reasons for that restriction. Finally, a license change requires the explicit approval by all co-authors. In this case, it is strongly recommended that you open an issue on your repo about the license change, and get all authors to accept the change in writing there.
I proposed an edit to your config so that the license info appears in the footer of the Jupyter Book, rather than a standard copyright notice. See: https://github.com/mikemorris12/UTCDW_Guidebook/pull/5
@mikemorris12 this is just to ask if you have any progress in considering a new license for your resource.
Hi @acocac and @labarba, thank you for pointing out the remaining license issue, and apologies for the delay. It took a few days for the other coauthors to get back to me on this matter.
After consideration, we have decided we are obligated to use the CC-BY-SA license. This is because the Guidebook borrows materials (Specifically Chapter 3.4) from Anderson and Smith (2021), which is licensed under CC-BY-SA (see the GitLab repository). This means we must use the same license.
I apologize for making what appeared to be a license change, that was merely me correcting two mistakes. One was indicating CC-BY on the Zenodo archive, and the second was using the CC-BY badge on the README page. The actual LICENSE file in the Guidebook repository was always for CC-BY-SA. Therefore I don't think we should need to open an issue about a license change, since the actual license never changed. I sincerely apologize for creating confusion about the license.
Regarding @labarba's comment that the code should be licensed separately, how should I go about doing that? Should I add a second LICENSE file to the repository? Or simply state in the README that the code materials are licensed under the MIT (for example) license? Thanks!
@mikemorris12 thanks for the update on this.
Re licenses, Github seems to accept multiple of them, you should follow the GitHub blog post: https://github.blog/changelog/2022-05-26-easily-discover-and-navigate-to-multiple-licenses-in-repositories/. It seems you should create a LICENSE file for the docs, and other LICENSE-CODE with the OSI-approved license for software. Please see an example in the repo of docs.github.com.
Let's wait @labarba thoughts.
Since it's been a week and we haven't yet heard from @labarba I decided to go ahead and add the LICENSE-CODE file to the repository. (using the MIT license), following @acocac's instructions. I also added the MIT license to the Zenodo archive. Is there any further action required? Thanks!
Since it's been a week and we haven't yet heard from @labarba I decided to go ahead and add the LICENSE-CODE file to the repository. (using the MIT license), following @acocac's instructions. I also added the MIT license to the Zenodo archive. Is there any further action required? Thanks!
It looks good to me. May I ask to release a new version e.g. 1.1.1 in GitHub to include the added LICENSE-CODE file? You should archive the new version in Zenodo too.
Submitting author: !--author-handle-->@mikemorris12<!--end-author-handle-- (Michael Morris) Repository: https://github.com/mikemorris12/UTCDW_Guidebook/ Branch with paper.md (empty if default branch): master Version: v1.1.1 Editor: !--editor-->@acocac<!--end-editor-- Reviewers: @aulemahal, @brian-rose Archive: 10.5281/zenodo.12785645 Paper kind: learning module
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
@aulemahal & @brian-rose, 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://openjournals.readthedocs.io/en/jose/reviewer_guidelines.html. Any questions/concerns please let @acocac 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 @aulemahal
📝 Checklist for @brian-rose