openjournals / joss-reviews

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

[REVIEW]: LorenzCycleToolkit: A Comprehensive Python Tool for Analyzing Atmospheric Energy Cycles #7139

Closed editorialbot closed 1 month ago

editorialbot commented 2 months ago

Submitting author: !--author-handle-->@daniloceano<!--end-author-handle-- (Danilo Couto de Souza) Repository: https://github.com/daniloceano/LorenzCycleToolkit Branch with paper.md (empty if default branch): joss-submission Version: v1.0.7 Editor: !--editor-->@observingClouds<!--end-editor-- Reviewers: @einaraz, @amylu00 Archive: 10.5281/zenodo.13765959

Status

status

Status badge code:

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

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

@einaraz & @amylu00, 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 @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 @amylu00

📝 Checklist for @einaraz

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

Software report:

github.com/AlDanial/cloc v 1.90  T=0.06 s (1607.5 files/s, 140587.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          30            997           1315           4072
CSV                             46              0              0            959
SVG                              1              1              1            472
reStructuredText                14            135            178            214
YAML                             4             46              0            204
Markdown                         4             53              0            148
TeX                              1             13              0            126
DOS Batch                        1              8              1             26
Bourne Shell                     1              7              0             12
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                           103           1264           1502           6242
-------------------------------------------------------------------------------

Commit count by author:

   478  daniloceano
    83  Danilo Couto d Souza
    24  Danilo Couto de Souza
editorialbot commented 2 months ago

Paper file info:

📄 Wordcount for paper.md is 1127

✅ The paper includes a Statement of need section

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

✅ OK DOIs

- 10.3402/tellusa.v7i2.8796 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: The Energy Cycle in Atmospheric Models
- No DOI given, and none found for title: The Nature and Theory of the General Circulation o...
- No DOI given, and none found for title: Data Science with Python and Dask
- No DOI given, and none found for title: xarray: ND Labeled Arrays and Datasets in Python
- No DOI given, and none found for title: pandas-dev/pandas: Pandas 1.0.5
- No DOI given, and none found for title: Array Programming with NumPy

❌ MISSING DOIs

- 10.1007/bf01029650 may be a valid DOI for title: The Global Energy Cycle of Stationary and Transien...
- 10.1029/2007gl029985 may be a valid DOI for title: Lorenz Energy Cycle of the Global Atmosphere Based...
- 10.1155/2013/485047 may be a valid DOI for title: A Global and Hemispherical Analysis of the Lorenz ...
- 10.1029/2011jd016217 may be a valid DOI for title: The Energy Cycle and Structural Evolution of Cyclo...
- 10.1256/smsqj.55307 may be a valid DOI for title: Quasi-Lagrangian Energetics of an Intense Mediterr...
- 10.1109/38.56302 may be a valid DOI for title: NetCDF: An Interface for Scientific Data Access
- 10.1175/bams-d-21-0125.1 may be a valid DOI for title: MetPy: A Meteorological Python Library for Data An...

❌ INVALID DOIs

- None
editorialbot commented 2 months ago

License info:

🟡 License found: GNU General Public License v3.0 (Check here for OSI approval)

editorialbot commented 2 months ago

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

observingClouds commented 2 months ago

Hi @daniloceano, This is the start of the review process. While the reviewers will be busy with reviewing your submission, could you please check the above mentioned references in your manuscript that do appear to miss a DOI and add it if possible. If no DOI is available for an entry, please add a URL and mention here why there is no DOI available, e.g. it is a reference to a poster. Cheers, Hauke

observingClouds commented 2 months ago

Hi @einaraz, @amylu00,

This is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains 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/7139 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 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.

daniloceano commented 2 months ago

Hi @daniloceano, This is the start of the review process. While the reviewers will be busy with reviewing your submission, could you please check the above mentioned references in your manuscript that do appear to miss a DOI and add it if possible. If no DOI is available for an entry, please add a URL and mention here why there is no DOI available, e.g. it is a reference to a poster. Cheers, Hauke

Hi Hauke,

Thank you for initiating the review process.

I have provided the DOIs for all references except for the following two, as they are books:

Since these are books, they do not have associated DOIs.

Please let me know if further adjustments are needed.

Best regards, Danilo

observingClouds commented 2 months ago

@daniloceano thanks for the update. Could you please add the ISBN for those books? We just like to have some permanent identifiers referenced to make it easier for readers to find the references later.

amylu00 commented 2 months ago

Review checklist for @amylu00

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

amylu00 commented 2 months ago

Hi @daniloceano! This seems like a cool repo, thank you for working on it. A few small comments/questions as I'm going through the review:

  1. What is the best way to run the test files? It seems like when I try running any of the test files, I get an error saying ModuleNotFoundError: No module named 'lorenzcycletoolkit.
  2. The second example ("Moving (Semi-Lagrangian) Framework Example") in your documentation required creating a file named track in the inputs folder. It would be great if that was already created within the repo and/or edit the documentation to say inputs/track file instead of _inputs/trackfile. This way, it is consistent with the first example where the user does not have to create the file themselves and also with respect to the naming of the files.
  3. The guidelines for any third party that would like to contribute to the software is clear. I would suggest including guidelines for those seeking support and also how and where to report any issues with the software.
daniloceano commented 2 months ago

@daniloceano thanks for the update. Could you please add the ISBN for those books? We just like to have some permanent identifiers referenced to make it easier for readers to find the references later.

Thank you for your feedback. I've added the ISBN for "Data Science with Python and Dask." However, "The Nature and Theory of the General Circulation of the Atmosphere" was published before ISBN numbers were introduced, so it does not have one. I was only able to find an ISBN for an Italian version of the book.

daniloceano commented 2 months ago

Hi @daniloceano! This seems like a cool repo, thank you for working on it. A few small comments/questions as I'm going through the review:

  1. What is the best way to run the test files? It seems like when I try running any of the test files, I get an error saying ModuleNotFoundError: No module named 'lorenzcycletoolkit.
  2. The second example ("Moving (Semi-Lagrangian) Framework Example") in your documentation required creating a file named track in the inputs folder. It would be great if that was already created within the repo and/or edit the documentation to say inputs/track file instead of _inputs/trackfile. This way, it is consistent with the first example where the user does not have to create the file themselves and also with respect to the naming of the files.
  3. The guidelines for any third party that would like to contribute to the software is clear. I would suggest including guidelines for those seeking support and also how and where to report any issues with the software.

Hi @amylu00,

Thank you so much for taking the time to review the repository! I appreciate your feedback and am happy to address your comments.

  1. ModuleNotFoundError: How are you currently running the test files? The updated documentation now indicates that the program should be run from the top-level directory of the project. This ensures that the modules are correctly found and executed.
  2. Track File Creation: Great point! There is already a sample track file included in the repository, and it can be easily copied to inputs/track. I’ve updated the documentation to reflect this, so users will know to use the sample file instead of creating it from scratch.
  3. Support and Issue Reporting: I’ve also added a section to the documentation outlining the best ways to seek support and report any issues with the software. This should make it easier for users to know where and how to get help.

Please let me know if you have any further questions or if there’s anything else you’d like me to clarify!

amylu00 commented 2 months ago
  1. ModuleNotFoundError: How are you currently running the test files? The updated documentation now indicates that the program should be run from the top-level directory of the project. This ensures that the modules are correctly found and executed.

I am running from the top-level directory and used python tests/test_ERA5_track.py. I tried running python lorenzcycletoolkit.py tests/test_ERA5_track.py as well but then I think that runs the lorenzcycletoolkit.py module instead of the tests/test_ERA5_track.py one.

Documentation looks good to me now, thanks for adding clarification!

daniloceano commented 2 months ago

I am running from the top-level directory and used python tests/test_ERA5_track.py. I tried running python lorenzcycletoolkit.py tests/test_ERA5_track.py as well but then I think that runs the lorenzcycletoolkit.py module instead of the tests/test_ERA5_track.py one.

Thanks for the update! Just to clarify so I can replicate the issue on my end—did you install the package using pip, or have you only cloned the repository and are running it directly from there?

amylu00 commented 2 months ago

I cloned the repo and am running it directly from there

daniloceano commented 2 months ago

Hi @amylu00,

Could you please run the following tests for me?

  1. From the top-level directory, run pytest.
  2. From the top-level directory, run python lorenzcycletoolkit.py.

If both commands work without issues, it seems everything is functioning correctly. I can then update the documentation to indicate that this is the proper way to run the tests.

Thanks!

amylu00 commented 2 months ago

@daniloceano both work, thank you so much!

@observingClouds I've gone through the checklist and feel like all checks are met. Please let me know if you need anything else on my end. Thanks!

einaraz commented 2 months ago

Review checklist for @einaraz

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

einaraz commented 1 month ago

Hi @daniloceano! Great job on the project and the software! I successfully installed and ran all the examples. I have a few comments and suggestions:

  1. The examples in the tutorial were clear. However, it might be helpful to move (or repeat) the definition of the Flags in the Tutorial and/or Usage section to make it more linear and clear for users when running the examples.
  2. The "contributing guide" page is currently empty.
  3. The README mentions that the license is MIT, but it appears that the actual license is GNU.
  4. I wonder if the README file should contain installation instructions. At least clarify that installation instructions are included in the documentation.
  5. Since the paper itself has no space for results, I think it would be great somewhere in the repo (README or in the results section of the documentation) to describe/interpret some of the outputs. Ofc that's only a suggestion, but I think it would make the project more appealing and help users understand the figures.
daniloceano commented 1 month ago

Hi @einaraz. Thank you very much for the feedback.

1. The examples in the tutorial were clear. However, it might be helpful to move (or repeat) the definition of the Flags in the Tutorial and/or Usage section to make it more linear and clear for users when running the examples.

I've moved the Flags section into the "Usage" section to make the flow more straightforward for users. Does this look better to you?

2. The "contributing guide" page is currently empty.

Could you please check again? The guide should now be accessible here: https://daniloceano.github.io/LorenzCycleToolkit/contributing.html

3. The README mentions that the license is MIT, but it appears that the actual license is GNU.

This has been corrected. Thanks for catching that!

4. I wonder if the README file should contain installation instructions. At least clarify that installation instructions are included in the documentation.

I’ve updated the README to mention that installation instructions can be found in the documentation.

5. Since the paper itself has no space for results, I think it would be great somewhere in the repo (README or in the results section of the documentation) to describe/interpret some of the outputs. Ofc that's only a suggestion, but I think it would make the project more appealing and help users understand the figures.

You’re right — interpreting Lorenz Energy Cycle outputs requires a solid understanding of atmospheric dynamics, and a full explanation would typically be found in textbooks. However, I’m currently preparing a submission of results generated using this program for a dedicated journal. Once the paper is published, I’ll link it in the results section of the documentation to provide further context and help future users interpret the outputs of LorenzCycleToolkit.

In the meantime, I encourage users to refer to the original Lorenz (1965) paper and the references in the documentation to better understand the energy cycle and its applications.

einaraz commented 1 month ago

Thank you for addressing my comments, @daniloceano! The proposed changes look good!

  1. I see you updated the doc files to add Flags under usage; however, the documentation was not updated. Could you please rebuild the page?
  2. Also, the link for contributing you provided in the reply directs to the correct place; the one linked on the README file, under Contributing, is trying to link directly to the file 'CONTRIBUTING.md' that does not exist in the directory.
  3. I think it's a great idea to direct users to your other publications and additional papers on the subject! A suggestions would be to add a "References" session at the bottom of the README file to include relevant materials.
daniloceano commented 1 month ago

Thank you for your feedback and suggestions!

1) I have rebuilt the page to reflect the updated documentation with the changes to the "Flags" section under "Usage."

2) I’ve corrected the link in the README.md. The link now points to the correct location in the documentation for contributing guidelines.

3) Regarding the references, thank you for the suggestion! I've included all relevant references and materials within the main documentation instead of the README.md. This keeps the README concise, while the documentation provides a more in-depth reference list.

Please let me know if further changes are needed!

einaraz commented 1 month ago

Thank you, @daniloceano! It all looks great to me! @observingClouds, I believe the project checks all the boxes! I have no further comments

observingClouds commented 1 month ago

@editorialbot check references

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

✅ OK DOIs

- 10.3402/tellusa.v7i2.8796 is OK
- 10.1007/bf01029650 is OK
- 10.1029/2007GL029985 is OK
- 10.1007/s00382-007-0303-4 is OK
- 10.1155/2013/485047 is OK
- 10.1029/2011JD016217 is OK
- 10.1002/qj.49712555309 is OK
- 10.1109/38.56302 is OK
- 10.1175/BAMS-D-21-0125.1 is OK
- 10.5334/jors.148 is OK
- 10.5281/zenodo.3898987 is OK
- 10.1038/s41586-020-2649-2 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: The Nature and Theory of the General Circulation o...
- No DOI given, and none found for title: Data Science with Python and Dask

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
observingClouds commented 1 month ago

Dear @amylu00, @einaraz, Thank you very much for your thorough and quick review. We really appreciate your time investment. I will now go ahead and will work together with @daniloceano to make the manuscript ready for acceptance. Cheers!

observingClouds commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

daniloceano commented 1 month ago

Hey @observingClouds, just wondering if everything’s ok or if there’s anything that is needed from my end at this point .

observingClouds commented 1 month ago

Hi @daniloceano, There are now a few last items that we need to do as mentioned in the checklist below. Please also have a look at my proposed changes to the manuscript. Cheers!

observingClouds commented 1 month ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

observingClouds commented 1 month ago

@daniloceano please create a new release of your software, archive it and provide here the version and DOI so I can create the links in the manuscript accordingly.

daniloceano commented 1 month ago

@observingClouds

Everything's done!

I have double-checked the authors and affiliations, created the v1.0.7 release on GitHub, and archived the release on Zenodo. The authors list, ORCIDs, and license in the Zenodo repository match those in the JOSS paper.

The Zenodo DOI is: 10.5281/zenodo.13765959

observingClouds commented 1 month ago

Thanks @daniloceano! Could you adjust the title of the zenodo archive so that it matches the paper title? Thanks!

observingClouds commented 1 month ago

@editorialbot set 10.5281/zenodo.13765959 as archive

editorialbot commented 1 month ago

Done! archive is now 10.5281/zenodo.13765959

observingClouds commented 1 month ago

@editorialbot set v1.0.7 as version

editorialbot commented 1 month ago

Done! version is now v1.0.7

observingClouds commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

daniloceano commented 1 month ago

Thanks @daniloceano! Could you adjust the title of the zenodo archive so that it matches the paper title? Thanks!

Done!

observingClouds commented 1 month ago

@editorialbot check references

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

✅ OK DOIs

- 10.3402/tellusa.v7i2.8796 is OK
- 10.1007/bf01029650 is OK
- 10.1029/2007GL029985 is OK
- 10.1007/s00382-007-0303-4 is OK
- 10.1155/2013/485047 is OK
- 10.1029/2011JD016217 is OK
- 10.1002/qj.49712555309 is OK
- 10.1109/38.56302 is OK
- 10.1175/BAMS-D-21-0125.1 is OK
- 10.5334/jors.148 is OK
- 10.5281/zenodo.3898987 is OK
- 10.1038/s41586-020-2649-2 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: The Nature and Theory of the General Circulation o...
- No DOI given, and none found for title: Data Science with Python and Dask

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
observingClouds commented 1 month ago

@editorialbot recommend-accept

editorialbot commented 1 month ago
Attempting dry run of processing paper acceptance...
editorialbot commented 1 month ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.3402/tellusa.v7i2.8796 is OK
- 10.1007/bf01029650 is OK
- 10.1029/2007GL029985 is OK
- 10.1007/s00382-007-0303-4 is OK
- 10.1155/2013/485047 is OK
- 10.1029/2011JD016217 is OK
- 10.1002/qj.49712555309 is OK
- 10.1109/38.56302 is OK
- 10.1175/BAMS-D-21-0125.1 is OK
- 10.5334/jors.148 is OK
- 10.5281/zenodo.3898987 is OK
- 10.1038/s41586-020-2649-2 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: The Nature and Theory of the General Circulation o...
- No DOI given, and none found for title: Data Science with Python and Dask

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
editorialbot commented 1 month ago

:wave: @openjournals/ese-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/5889, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

observingClouds commented 1 month ago

@daniloceano thank you very much for your quick action. I have now recommended this submission for acceptance. Congratulations! The topical editor will soon have a close look and finalize the publication.

Also a big thank you to @amylu00 and @einaraz for their quick and through reviews. Without you this would not be possible.