search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
721 stars 38 forks source link

[REVIEW]: scores: A Python package for verifying and evaluating models and predictions with xarray #6889

Closed editorialbot closed 4 months ago

editorialbot commented 5 months ago

Submitting author: !--author-handle-->@tennlee<!--end-author-handle-- (Tennessee Leeuwenburg) Repository: https://github.com/nci/scores/ Branch with paper.md (empty if default branch): 528-joss-paper-submission Version: 0.9.3 Editor: !--editor-->@crvernon<!--end-editor-- Reviewers: @calebweinreb, @savente93 Archive: 10.5281/zenodo.12697242

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/c51b4a6def315f7b9789983663677468"><img src="https://joss.theoj.org/papers/c51b4a6def315f7b9789983663677468/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/c51b4a6def315f7b9789983663677468/status.svg)](https://joss.theoj.org/papers/c51b4a6def315f7b9789983663677468)

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

@calebweinreb & @savente93, 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:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @crvernon 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 @savente93

📝 Checklist for @calebweinreb

editorialbot commented 5 months 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
editorialbot commented 5 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.21105/joss.02781 is OK
- 10.1002/met.1732 is OK
- 10.5281/zenodo.5173153 is OK
- 10.1111/rssb.12154 is OK
- 10.1002/qj.4266 is OK
- 10.1198/tech.2011.10136 is OK
- 10.1016/S0169-2070(96)00719-4 is OK
- 10.5334/jors.148 is OK
- 10.3386/t0169 is OK
- 10.1175/BAMS-D-19-0093.1 is OK
- 10.22499/4.0021 is OK
- 10.1175/waf-d-19-0248.1 is OK
- 10.1002/qj.4206 is OK
- 10.1214/21-ejs1957 is OK
- 10.1198/jbes.2010.08110 is OK
- 10.1175/waf-d-23-0201.1 is OK
- 10.1175/bams-d-22-0253.1 is OK
- 10.1073/pnas.2016191118 is OK
- 10.1071/es21010 is OK
- 10.48550/arXiv.2404.18429 is OK
- 10.1002/qj.2270 is OK
- 10.1198/016214506000001437 is OK
- 10.5194/gmd-12-4185-2019 is OK
- 10.1002/qj.4461 is OK
- 10.5281/zenodo.10957263 is OK
- 10.25080/Majora-92bf1922-00a is OK
- 10.5281/zenodo.3764117 is OK
- 10.5065/D6H70CW6 is OK
- 10.11578/dc.20180330.1 is OK
- 10.5281/zenodo.3773450 is OK
- 10.1175/1520-0493(1950)078<0001:vofeit>2.0.co;2 is OK
- 10.1126/science.ns-4.93.453.b is OK
- 10.1175/WAF-D-10-05030.1 is OK
- 10.1175/2007MWR2123.1 is OK

MISSING DOIs

- No DOI given, and none found for title: Assessing calibration when predictive distribution...
- No DOI given, and none found for title: Dask: Library for dynamic task scheduling
- No DOI given, and none found for title: WMO No. 306 FM 92 GRIB (edition 2)
- No DOI given, and none found for title: Jupyter Interactive Notebook
- No DOI given, and none found for title: Finley’s tornado predictions

INVALID DOIs

- None
editorialbot commented 5 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.18 s (696.5 files/s, 260796.3 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Python                           75           2549           4625          11655
Markdown                         17            303              0           1356
Jupyter Notebook                 22              0          23900            891
TeX                               1              1              0            455
YAML                              6             19             21            129
TOML                              1             18              2            122
Bourne Again Shell                1              2              4              3
--------------------------------------------------------------------------------
SUM:                            123           2892          28552          14611
--------------------------------------------------------------------------------

Commit count by author:

   161  Tennessee Leeuwenburg
    38  Stephanie Chong
    27  Nicholas Loveday
    22  Steph-Chong
    16  tennlee
    15  Aidan Griffiths
    11  Harrison Cook
     9  nicholas loveday
     8  Deryn
     7  John Sharples
     5  Nikeeth Ramanathan
     5  nicholasloveday
     3  Beth Ebert
     3  agriffit
     3  dependabot[bot]
     3  reza-armuei
     2  Maree Carroll
     2  nikeethr
     1  rob-taggart
editorialbot commented 5 months ago

Paper file info:

📄 Wordcount for paper.md is 1390

✅ The paper includes a Statement of need section

editorialbot commented 5 months ago

License info:

🟡 License found: Other (Check here for OSI approval)

editorialbot commented 5 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

crvernon commented 5 months ago

👋 @tennlee, @calebweinreb, and @savente93 - This is the review thread for the paper. All of our communications will happen here from now on.

Please read the "Reviewer instructions & questions" in the first comment above.

Both reviewers have checklists at the top of this thread (in that first comment) with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

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 https://github.com/openjournals/joss-reviews/issues/6889 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for the review process to be completed within about 4-6 weeks but please make a start well ahead of this as JOSS reviews are by their nature iterative and any early feedback you may be able to provide to the author will be very helpful in meeting this schedule.

tennlee commented 5 months ago

Thanks all. I note the first comment from editorialbot refers to version 0.9.0. This comment is just to explain that we have now released version 0.9.1, which contains some important updates.

savente93 commented 4 months ago

Review checklist for @savente93

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

savente93 commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

savente93 commented 4 months ago

I've gone through a lot of the auxiliary stuff and made some (hopefully useful) remarks, I'll continue my review of the functionality etc. etc. sometime later this week.

tennlee commented 4 months ago

@editorialbot commands

editorialbot commented 4 months ago

Hello @tennlee, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# 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

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers
tennlee commented 4 months ago

@editorialbot check repository

Running this to see if the automated check is now happy with the changes to the license specification.

editorialbot commented 4 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.17 s (704.3 files/s, 263709.3 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Python                           75           2549           4625          11655
Markdown                         17            303              0           1356
Jupyter Notebook                 22              0          23900            891
TeX                               1              1              0            455
YAML                              6             19             21            129
TOML                              1             18              2            122
Bourne Again Shell                1              2              4              3
--------------------------------------------------------------------------------
SUM:                            123           2892          28552          14611
--------------------------------------------------------------------------------

Commit count by author:

   161  Tennessee Leeuwenburg
    38  Stephanie Chong
    27  Nicholas Loveday
    22  Steph-Chong
    16  tennlee
    15  Aidan Griffiths
    11  Harrison Cook
     9  nicholas loveday
     8  Deryn
     7  John Sharples
     5  Nikeeth Ramanathan
     5  nicholasloveday
     3  Beth Ebert
     3  agriffit
     3  dependabot[bot]
     3  reza-armuei
     2  Maree Carroll
     2  nikeethr
     1  rob-taggart
editorialbot commented 4 months ago

Paper file info:

📄 Wordcount for paper.md is 1390

✅ The paper includes a Statement of need section

editorialbot commented 4 months ago

License info:

🟡 License found: Other (Check here for OSI approval)

tennlee commented 4 months ago

@editorialbot check repository

I suspect the bot did not pick up the license from the default branch. I have copied the new license file into the same branch as contains paper.md. Running the check again.

editorialbot commented 4 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.17 s (707.5 files/s, 264921.6 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Python                           75           2549           4625          11655
Markdown                         17            303              0           1356
Jupyter Notebook                 22              0          23900            891
TeX                               1              1              0            455
YAML                              6             19             21            129
TOML                              1             18              2            122
Bourne Again Shell                1              2              4              3
--------------------------------------------------------------------------------
SUM:                            123           2892          28552          14611
--------------------------------------------------------------------------------

Commit count by author:

   162  Tennessee Leeuwenburg
    38  Stephanie Chong
    27  Nicholas Loveday
    22  Steph-Chong
    16  tennlee
    15  Aidan Griffiths
    11  Harrison Cook
     9  nicholas loveday
     8  Deryn
     7  John Sharples
     5  Nikeeth Ramanathan
     5  nicholasloveday
     3  Beth Ebert
     3  agriffit
     3  dependabot[bot]
     3  reza-armuei
     2  Maree Carroll
     2  nikeethr
     1  rob-taggart
editorialbot commented 4 months ago

Paper file info:

📄 Wordcount for paper.md is 1390

✅ The paper includes a Statement of need section

editorialbot commented 4 months ago

License info:

✅ License found: Apache License 2.0 (Valid open source OSI approved license)

tennlee commented 4 months ago

Note for reviewers - GitHub's security scanner, dependabot, noted two dependencies which needed updating. These both pertained to the "binder" integration and I have updated the versions accordingly on our default branch. The changes are unrelated to the main dependencies of scores and are unlikely to affect anyone not using binder, but I am happy to update branch "528-joss-paper-submission" if that is wanted, just let me know if you would like that.

savente93 commented 4 months ago

I won't speak for the other reviewer, but for me this is not necessary. Thanks for letting us know.

savente93 commented 4 months ago

@tennlee @crvernon I am done with my initial review. I've made issues on the repository with some suggestions for improvements. I think the only issue I'd like to see addressed for giving scores a passing mark is https://github.com/nci/scores/issues/549 all the other ones are optional.

As a general remark, the documentation of scores is excellent. I do consider <400 commits over 2 (ish) years a somewhat immature package, but it definitely meets the criteria. If it fulfilled a need I had I would feel comfortable giving this package a serious try.

I'll be leaving things off here for now but I should be available for comments and questions whenever necessary.

crvernon commented 4 months ago

Thanks @savente93!

tennlee commented 4 months ago

Hi all. I have rebased the branch "528-joss-paper-submission" which contains the paper.md file against "develop" which is our default and development branch. This fixes some test failures arising from shifting versions in the dependencies, and brings in various documentation updates (including some of those recommended by the reviewers). I was unclear whether to keep the 528 branch up to date or not, but in light of failing tests occurring, it seemed the most practical choice. I can revert this situation if requested. Thanks. Note - the easiest way to review the latest documentation is via https://scores.readthedocs.io/en/develop/ rather than reading through the source markdown, though of course that's also something people are welcome to do.

tennlee commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

tennlee commented 4 months ago

@savente93 @calebweinreb We have updated the paper (paper.md) on branch 528-joss-paper-submission so it is now ready for further review. We have also made some updates to the documentation on the develop branch which have not yet made their way to main (see https://scores.readthedocs.io/en/latest).

savente93 commented 4 months ago

I've reviewed the updated paper, and I'm happy with what has been provided, so as far as I'm concerned the scores submission is ready to be accepted. There are a few things I've mentioned yet, but those are not necessary for acceptance and might take a bit longer, so I'll just leave that up to the authors (I believe they've already made significant progress on them). @crvernon I've completed my checklist, is anything else needed from my side?

crvernon commented 4 months ago

:wave: @calebweinreb - can you provide an estimate on when you think you may be able to conduct your portion of the review? Thanks!

calebweinreb commented 4 months ago

Hi Chris,

Sorry for the delay! I expect to finish and provide comments this week.

crvernon commented 4 months ago

Thanks so much @calebweinreb! Sounds great!

calebweinreb commented 4 months ago

Review checklist for @calebweinreb

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

calebweinreb commented 4 months ago

@tennlee @crvernon

I have gone through the paper/docs and installed/tested the package. Overall I am super impressed with the documentation and the completeness and polish of the package. I raised a few issues related to the manuscript and documentation. The only blocking issue is https://github.com/nci/scores/issues/577, which must be addressed or rebutted before I am willing to approve "summary" and "statement of need" in the checklist. https://github.com/nci/scores/issues/575 and https://github.com/nci/scores/issues/574 seem like easy wins to me. I think https://github.com/nci/scores/issues/576 and https://github.com/nci/scores/issues/578 could improve clarity/accessibility of the tool, but as an outsider to the field, I am not familiar with the target audience and therefore defer to the developers on whether these additions would be broadly helpful.

crvernon commented 4 months ago

Thanks! @calebweinreb!

tennlee commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

tennlee commented 4 months ago

@calebweinreb here is a revised version of the paper which we believe addresses your feedback on https://github.com/nci/scores/issues/577. Let us know if this seems suitable.

calebweinreb commented 4 months ago

@tennlee the new draft looks great! It seems like the reduce/preserve tutorial and jupyter instructions are still in the works. I'll await your updates on those and then close out the checklist.

tennlee commented 4 months ago

@editorialbot generate preprint

editorialbot commented 4 months ago

:page_facing_up: Preprint file created: Find it here in the Artifacts list :page_facing_up:

tennlee commented 4 months ago

@calebweinreb Thank you very much for your helpful feedback :)

We have now resolved nci/scores#577, which you said earlier was the only blocking issue. We have also resolved nci/scores#575 and nci/scores#574 which you raised.

As noted in issue nci/scores#578, this will need more time to resolve. My understanding is that you do not believe it to be a blocking issue.

Regarding nci/scores#576, I have posted a clarification on the issue thread, to the effect that while it would be helpful for some new users or those outside the field, I don’t think it is an issue for the main audience of scores. As such, I do intend to address it, but I believe it can be done subsequent to publication. This will allow more time for the development and review of the tutorial.

Can you please let me know whether you are happy for publication to proceed as-is, or whether nci/scores#576 should be addressed first.

tennlee commented 4 months ago

@calebweinreb We have now added a tutorial regarding dimensions handling, to address your issue nci/scores#576 (reduce/preserve handling).

I think this now addresses everything required.

calebweinreb commented 4 months ago

I agree. At this point I would support acceptance of the submission. Congrats @tennlee and authors on a great package!

crvernon commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

crvernon commented 4 months ago

@editorialbot check references

editorialbot commented 4 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.21105/joss.02781 is OK
- 10.1002/met.1732 is OK
- 10.5281/zenodo.5173153 is OK
- 10.1111/rssb.12154 is OK
- 10.1002/qj.4266 is OK
- 10.1198/tech.2011.10136 is OK
- 10.1016/S0169-2070(96)00719-4 is OK
- 10.5334/jors.148 is OK
- 10.1080/07350015.1995.10524599 is OK
- 10.1175/BAMS-D-19-0093.1 is OK
- 10.22499/4.0021 is OK
- 10.1175/waf-d-19-0248.1 is OK
- 10.1002/qj.4206 is OK
- 10.1214/21-ejs1957 is OK
- 10.1198/jbes.2010.08110 is OK
- 10.1175/waf-d-23-0201.1 is OK
- 10.1175/bams-d-22-0253.1 is OK
- 10.1073/pnas.2016191118 is OK
- 10.1071/es21010 is OK
- 10.48550/arXiv.2404.18429 is OK
- 10.1002/qj.2270 is OK
- 10.1198/016214506000001437 is OK
- 10.5194/gmd-12-4185-2019 is OK
- 10.1002/qj.4461 is OK
- 10.5281/zenodo.10957263 is OK
- 10.25080/Majora-92bf1922-00a is OK
- 10.5281/zenodo.3764117 is OK
- 10.5065/D6H70CW6 is OK
- 10.11578/dc.20180330.1 is OK
- 10.5281/zenodo.3773450 is OK
- 10.1175/1520-0493(1950)078<0001:vofeit>2.0.co;2 is OK
- 10.1126/science.ns-4.93.453.b is OK
- 10.1175/WAF-D-10-05030.1 is OK
- 10.1175/2007MWR2123.1 is OK

MISSING DOIs

- No DOI given, and none found for title: Assessing calibration when predictive distribution...
- No DOI given, and none found for title: Dask: Library for dynamic task scheduling
- No DOI given, and none found for title: WMO No. 306 FM 92 GRIB (edition 2)
- No DOI given, and none found for title: Jupyter Interactive Notebook
- No DOI given, and none found for title: Finley’s tornado predictions

INVALID DOIs

- None