[REVIEW]: AgriFoodPy: a package for modelling food systems

editorialbot commented 7 months ago

Submitting author: !--author-handle-->@jucordero@crvernon<!--end-editor-- Reviewers: @kanishkan91, @jsun Archive: 10.5281/zenodo.11236802

Status

Status badge code:

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

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

@kanishkan91 & @jsun, 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 @jsun

📝 Checklist for @kanishkan91

editorialbot commented 7 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 7 months ago

Software report:

github.com/AlDanial/cloc v 1.88  T=0.02 s (808.7 files/s, 74696.9 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
XML                              1              0              0            493
Python                          13            171            289            392
Markdown                         2             43              0            202
TeX                              1              9              0            116
YAML                             2              3              4             33
-------------------------------------------------------------------------------
SUM:                            19            226            293           1236
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 7 months ago

Wordcount for paper.md is 1179

editorialbot commented 7 months ago

Failed to discover a valid open source license

editorialbot commented 7 months ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1126/science.aaq0216 is OK
- 10.5334/jors.148 is OK
- 10.5285/d6f8c045-521b-476e-b0d6-b3b97715c138 is OK
- 10.5281/zenodo.3946761 is OK
- 10.5194/gmd-14-3007-2021 is OK
- 10.1016/j.envsoft.2014.07.009 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 7 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 7 months ago

👋 @jucordero , @kanishkan91 , and @jsun - 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/6305 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.

jsun commented 7 months ago

Review checklist for @jsun

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/FixOurFood/AgriFoodPy/?
[x] License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@jucordero) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

jsun commented 6 months ago

Hi, @jucordero, @crvernon, I have not yet finished checking all the items; but I have written below with my comments on the parts I have checked so far.

Documentation

The documentation (probably README.md) gave several examples, however, I think more documentation is required for this package. For example, what kind of functions/methods are implemented in this package, what are the arguments of the function, ...

Software paper

It is recommended to write a more concise overview of the software in Summary section. Please consider writing about detailed features of the software and features to be implemented in the future in Features section.
The background was well-written, but more emphasis should be placed on what exactly this software can do and what it can do better than existing software.
The structure of the paper needs proofreading.

Additional comments

In the paper, the authors write a lot about future releases and implementations. This underlines that the development of the package is incomplete. I suggest that author implement these features during this review process.

jucordero commented 6 months ago

@jsun thanks for the comments. I'll provide a more comprehensive response soon. @crvernon Regarding the last bullet point: In general, is it okay to push significant changes to the main branch while the review is in process? We have been working on the features mentioned as the future work but haven't merged from my fork yet.

crvernon commented 6 months ago

@jucordero yes, no problem at all to update the main branch during review as long as those changes have been made known to reviewers in this thread. These changes should also not interfere with any current features that the reviewers are currently reviewing. So you may want to check with them directly as well. Anytime you link to this thread in your pull requests or issues it will show up here so all can track progress.

crvernon commented 6 months ago

:wave: @jucordero , @kanishkan91 , and @jsun

Could you all provide a short few sentences/bullet points on how things are going with this review? Thanks and keep up the great work!

jucordero commented 5 months ago

Hi @crvernon

@jsun has made a few suggestions on the documentation and software paper which we have been addressing. Simultaneously we have been working on one of the main functionalities which was missing in the first release, but haven't discussed with the reviewers yet on whether it is fine to merge this right now.

I haven't heard from @kanishkan91

Happy Easter!

crvernon commented 5 months ago

Thanks @jucordero, Happy Easter as well! I'll touch base with @kanishkan91, I know he had some other things he had to prioritize but should be back in the thread here soon.

kanishkan91 commented 5 months ago

@crvernon @jucordero - My apologies for the delay. Will get this review done by this weekend.

crvernon commented 5 months ago

No problem @kanishkan91 and thank you!

kanishkan91 commented 5 months ago

@jucordero @crvernon - I just got done with most of my review. Apologies for the long delay! I opened several issues related to the same. I'm also summarizing below.

Summary- AgriFoodPy harmonizes data from the FAO and allows users to manipulate and run several models for several variables effectively. Anyone who is familiar with the idosyncracies of the FAO data would really appreciate this package. However, there are a few issues that I noted that I mention below (I have opened issues as well). After the same are addressed, I would be happy to recommend publication.

Issues

Edits required to run the README examples- Some README examples did not work as expected. See here- https://github.com/FixOurFood/AgriFoodPy/issues/67. While some require some updates and more explicit installation instructions- https://github.com/FixOurFood/AgriFoodPy/issues/66. Finally, it seems some examples require explicit import of matplotlib and its usage- https://github.com/FixOurFood/AgriFoodPy/issues/68. I think these should be easy to fix overall, but would need to be fixed for new users.
Reforestation example does not work- See the issue I opened for the same here- https://github.com/FixOurFood/AgriFoodPy/issues/70. This one also seems like a simple fix. Though I recommend it be fixed.
Where are the docs/ Add explicit link to the docs- There are several very valuable classes and methods available in this package. However, there is no documentation available for the same (other than the README). I saw that the authors have generated docs via sphinx. Can the htmls generated by sphinx be recorded in the "About" section of this repo? Also, sphinx usually documents every class and method, but I could not find these. These should be made available or hosted on GitHub docs. https://github.com/FixOurFood/AgriFoodPy/issues/69
Add link to examples- At first glance it seemed that there were no concrete examples available for this package (other than the README). However, that is not true. Authors have put together concrete examples here- https://github.com/FixOurFood/AgriFoodPy/tree/main/examples/modules. But there is no documenation for these . Can the authors add that and also link the same in the README. See this issue- https://github.com/FixOurFood/AgriFoodPy/issues/71

kanishkan91 commented 5 months ago

Review checklist for @kanishkan91

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/FixOurFood/AgriFoodPy/?
[x] License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@jucordero) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

jucordero commented 5 months ago

@kanishkan91 Thanks for the review! I'll start working on the issues as soon as posible.

crvernon commented 5 months ago

👋 @jucordero, @kanishkan91, and @jsun - could you provide a short update to the progress of the review here please? Thanks and keep up the great work!

crvernon commented 4 months ago

👋 @jucordero, @kanishkan91, and @jsun - just a reminder...could you provide a short update to the progress of the review here please? Thanks and keep up the great work!

jsun commented 4 months ago

hi, I'm just waiting for a response to my previous comment, https://github.com/openjournals/joss-reviews/issues/6305#issuecomment-1968127290.

kanishkan91 commented 4 months ago

@crvernon Looks like @jucordero has started addressing my issues. Once those are done, I'l finish up my checklist

jucordero commented 4 months ago

Hi @crvernon, @jsun and @kanishkan91 Once again, thank you very much for your comments and suggestions. These have helped us improve the accessibility to the existing documentation, the examples and the quality of the experience for first users when landing on the package repository. Apologies for the delay of this response. It has been a few very busy weeks lately.

@jsun Documentation Documentation for the package in the form of an auto-API document already exists for the package. It is automatically generated and available in readthedocs. We have included links to it both in the README and the about section of the repository to make it more visible. These have been addressed in https://github.com/FixOurFood/AgriFoodPy/pull/75

@kanishkan91

In PR#74 we have fixed the issue with the reforestation example not running properly. This was caused by an object imported from the agrifoodpy_data package, which changed from a DataArray to a Dataset. This fixes Issue 70.
In PR#75 we made several changes to the README to address some of the issues you have raised. We added a new section to point to the documentation (Issue 69) and examples (Issue 71). As mentioned above we also added a link to the readthedocs documentation in the about section of the repository.
Made small changes to the code snippet in the README, including explicit instructions to install the optional agrifoodpy_data package and statements to correctly display the generated plots. This addresses Issue 66 and Issue 68

There are still a few outstanding tasks which we haven't completed but are still working on.

Edits to the software paper suggested by @jsun: We are in the process of editing the summary and adding a "Functionality" section to the paper. This should be finished early next week.
Version requirements for the dependencies, raised by @kanishkan91: I'm not entirely sure on what criteria to use to define the minimum required versions for each dependency. Still investigating this.

As soon as we complete those tasks I'll ping you in this thread.

kanishkan91 commented 4 months ago

@jucordero On my version requirements, you can set this to >= whatever you have on your machine. Idea here is that a user should not install an older version of the dependency.

FYI @crvernon I have finished up my checklist. Assuming @jucordero addresses the above, this should be good to go.

crvernon commented 4 months ago

👋 @jucordero, @kanishkan91, and @jsun - how are we doing with this review? It look like you are close to the finish line! Could you give me a short update to what is left? Thanks!

jsun commented 4 months ago

I'll finish my review after checking the updated manuscript :)

jucordero 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:

jucordero commented 4 months ago

@jsun I have updated the manuscript.

Made a few changes to the summary to make it more concise.
Added a features section. Also rewrote a few paragraphs to remove some redundant information
Fixed some typos and changed to a more consistent use of concepts.

@kanishkan91 In PR#77 I have added version requirements to the modules to set the conda environment. Also took the opportunity to update deprecated GitHub actions which were causing issues with the unit tests.

jsun commented 4 months ago

@jucordero Thanks for considering my comments.

Hi, @crvernon , Ive just finished my review.

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.1126/science.aaq0216 is OK
- 10.5334/jors.148 is OK
- 10.5285/d6f8c045-521b-476e-b0d6-b3b97715c138 is OK
- 10.5281/zenodo.3946761 is OK
- 10.5194/gmd-14-3007-2021 is OK
- 10.1016/j.envsoft.2014.07.009 is OK

MISSING DOIs

- No DOI given, and none found for title: FAO. FAOSTAT. License: CC BY-NC-SA 3.0 IGO. Extrac...
- No DOI given, and none found for title: United Nations, Department of Economic and Social ...
- No DOI given, and none found for title: Rasterio: geospatial raster I/O for Python program...

INVALID DOIs

- None

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

:wave: @jucordero - I am conducting my pass now and have a few comments that need to be addressed before moving forward:

In the paper:

[x] LINES 24-25: The statement "...layer between for data sources..." seems off. Should the "for" be removed?
[x] LINE 59: The statement "....the importance of food production on climate change." reads a bit odd. How about "...the importance of accounting for the impacts of food production on climate change." or something like that.
[x] LINE 75: The word "The" does not need to be capitalized in "...The Environmental..."
[x] There is an issue with the formatting of your Gillies et al. paper which has two hyphens after year 2013. This will likely need to be corrected in your bib file.
[ ] Please check to see if the UN 2022 document you site has a DOI or weblink available.

Once you address these I'll provide further instruction on how we can move forward with getting this submission ready for acceptance. Thanks!

jucordero commented 4 months ago

Thanks for your comments, @crvernon.

I have made the following changes to the paper

Removed stray 'for' in line 24-25
In line 59, opted for a more straightforward wording. Used "have reported the impact of food production on climate change" instead.
Removed capitalization of the word 'The' in line 75
Edited the bibtex for the Gilles et al. Rasterio paper. It is a bit odd but that is the official bibtex entry they have in their repository citation guidelines.
I have changed the reference to the FAOSTAT dataset to point to the latest published report, which does have a corresponding DOI. This is probably the correct way to cite the Food Balance Sheets anyways.

jucordero 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

@jucordero thank you! Concerning the following:

I have changed the reference to the FAOSTAT dataset to point to the latest published report, which does have a corresponding DOI. This is probably the correct way to cite the Food Balance Sheets anyways.

I still see the UN 2022 reference in the paper. Did you mean to change it out elsewhere as well?

jucordero commented 4 months ago

@crvernon My bad, I changed the FAOSTAT reference, but not the UN population one. Couldn't fin a DOI for that particular report, but there is a web portal (https://population.un.org/wpp/) where the document is linked. Should I include that one to the citation?

crvernon commented 4 months ago

@jucordero no problem. Yes, I think including that link is better than not having one at all if it matches the citation standards. Just let me know when it is done so we can move forward. Thanks!

jucordero commented 4 months ago

@crvernon Done! I have just added the url to the bib entry.

jucordero 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.1126/science.aaq0216 is OK
- 10.5334/jors.148 is OK
- 10.4060/cc8088en is OK
- 10.5285/d6f8c045-521b-476e-b0d6-b3b97715c138 is OK
- 10.5281/zenodo.3946761 is OK
- 10.5194/gmd-14-3007-2021 is OK
- 10.1016/j.envsoft.2014.07.009 is OK

MISSING DOIs

- No DOI given, and none found for title: United Nations, Department of Economic and Social ...
- No DOI given, and none found for title: Rasterio: geospatial raster I/O for Python program...

INVALID DOIs

- None

crvernon commented 4 months ago

👋 @jucordero - we are almost there! Next is just setting up the archive for your new release.

We want to make sure the archival has the correct metadata that JOSS requires. This includes a title that matches the paper title and a correct author list.

So here is what we have left to do:

[x] Conduct a GitHub release of the current reviewed version of the software and archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository). Please ensure that the software archive uses the same license as the license you have posted on GitHub.
[x] Check the archival deposit (e.g., in Zenodo) to ensure it has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list matches the people and order in your paper). You may also add the authors' ORCID.
[x] Please respond with the 1) version and 2) DOI of the archived version here

I can then move forward with accepting the submission.

jucordero commented 4 months ago

@crvernon I have just created Release v0.1.2 and its corresponding entry in Zenodo

crvernon commented 4 months ago

@editorialbot set v0.1.2 as version

openjournals / joss-reviews