openjournals / joss-reviews

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

[REVIEW]: PROFFASTpylot: Running PROFFAST with Python #6481

Closed editorialbot closed 4 months ago

editorialbot commented 6 months ago

Submitting author: !--author-handle-->@feldl<!--end-author-handle-- (Lena Feld) Repository: https://gitlab.eudat.eu/coccon-kit/proffastpylot Branch with paper.md (empty if default branch): joss Version: v1.3.2 Editor: !--editor-->@rwegener2<!--end-editor-- Reviewers: @usethedata, @simonom Archive: 10.5281/zenodo.11035671

Status

status

Status badge code:

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

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

@usethedata & @simonom, 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 @rwegener2 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 @usethedata

πŸ“ Checklist for @simonom

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

Software report:

github.com/AlDanial/cloc v 1.90  T=0.03 s (1145.7 files/s, 162930.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                           8            395            655           1939
Markdown                        14            199              0            662
CSS                              1             53             38            337
TeX                              1             31              0            333
CSV                              2              0              0            249
YAML                             9             92            283             89
reStructuredText                 2             46             55             35
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            39            828           1039           3679
-------------------------------------------------------------------------------

Commit count by author:

   260  Lena Feld
   190  Benedikt Herkommer
     7  Jasmin Vestner
     1  benedikt.herkommer
     1  darko.dubravica
     1  lena.feld
editorialbot commented 6 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5194/amt-8-3059-2015 is OK
- 10.5194/acp-19-3271-2019 is OK
- 10.5194/amt-12-1513-2019 is OK
- 10.5194/amt-13-4751-2020 is OK
- 10.3390/rs13173395 is OK
- 10.5194/amt-2021-237 is OK
- 10.5194/amt-15-2433-2022 is OK
- 10.5194/acp-22-295-2022 is OK
- 10.5194/amt-9-2303-2016 is OK
- 10.5194/amt-14-1111-2021 is OK
- 10.21105/joss.05131 is OK
- 10.5194/amt-4-47-2011 is OK
- 10.5445/IR/1000159168 is OK
- 10.5194/amt-11-2173-2018 is OK
- 10.5194/amt-14-5887-2021 is OK
- 10.14291/TCCON.GGG2014.DOCUMENTATION.R0/1221662 is OK
- 10.5194/amt-13-4791-2020 is OK

MISSING DOIs

- No DOI given, and none found for title: COCCON Data Processing
- No DOI given, and none found for title: Using a portable FTIR spectrometer for checking th...

INVALID DOIs

- None
editorialbot commented 6 months ago

Paper file info:

πŸ“„ Wordcount for paper.md is 1082

βœ… The paper includes a Statement of need section

editorialbot commented 6 months ago

License info:

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

editorialbot commented 6 months ago

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

rwegener2 commented 6 months ago

πŸ‘‹πŸΌ @feldl @usethedata @simonom 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 openjournals/joss-reviews#REVIEW_NUMBER 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 (@rwegener2) if you have any questions/concerns.

usethedata commented 6 months ago

Review checklist for @usethedata

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

usethedata commented 6 months ago

@feldl -- I think there's an opportunity to provide a bit more background in either the Readme in the repository or in the documentation for the statement of need (what problem does this solve). The paper does a very good job of this, but in going over the readme.md and documentation, I was a couple of links deep (getting into the PROFFAST documentation) before I had any idea about what this software does. I like the summary in your paper, so one option would be to take that summary and put it in as a background paragraph in the documentation. Doing so might also increase the chances of discovery of this package via typical internet search engines.

usethedata commented 6 months ago

@feldl -- In checking the references, the Alberti et al (2021) reference (third one in the software paper) is pointing to https://doi.org/10.5194/amt-2021-237 -- but that's a preprint. A bit of searching led to https://doi.org/10.5194/amt-15-2199-2022 which appears to be the published version of that preprint. Can you double check that I'm correct and update the reference accordingly?

feldl commented 6 months ago

Dear @usethedata, Thank you for your constructive comments. You were right about the preprint: I have corrected the citation of Alberti et al. and updated the references. As you suggested, we have added the summary of the paper.md to our documentation. This is a very good idea, I can see that it is a bit confusing to look at the PROFFASTpylot documentation if you don't know about PROFFAST.

feldl commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

simonom commented 6 months ago

Review checklist for @simonom

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

simonom commented 5 months ago

Dear @feldl, I have some comments regarding installation:

i) can the package be installed on macOS systems? If not, then please state which operating systems/platforms the software is suitable for (perhaps beneath the Installation Title in the documentation, where the python specification is given)

ii) I found that I needed to install the gfortran package before PROFFAST could be installed, if this is generally true, then I suggest it be stated in the installation instructions

iii) In step 2 of installation instructions consider telling Linux users they must cd into the prf folder prior to running the installation script

iv) Step 4 of installation instructions says run.py can be used, but it doesn't say to move into the example folder first. The instruction is repeated in step 6 with direction to move into the example folder, so I suggest removing the instruction related to run.py in step 4.

I hope this is helpful.

simonom commented 5 months ago

@feldl and @rwegener2, I find this paper and software well prepared. Before I recommend publication, I just need a response and possible resolution on the minor points I provide above. Thanks.

rwegener2 commented 5 months ago

@usethedata Just checking in to see how your review is going. Feel free to reach out if you have any questions!

usethedata commented 5 months ago

@rwegener2 I got slammed with quarterly planning activities and travel. I'm planning to finish up this weekend, to see if I see anything different from what @simonom has already found with the installation issues. Testing installation and execution is all that's left on my list, I think.

usethedata commented 5 months ago

I concur with the comments made by @simonom above. I concur that this paper is ready for publication once the minor points that @simonom has identified and I've confirmed are addressed.

Specifically: i) It is probably possible to install the software on a Mac system, but there are challenges that I was not able to work around with a moderate amount of searching and experimentation (see notes below). ii) concur. For a linux system, gfortran is a prerequisite. It was installed on one of my test systems, but not a vanilla RHEL 8 box I was working with. iii) concur. iv) concur.

specifically, on a Mac, one needs gfortran, which can be installed using Homebrew (e.g. brew install gcc). macOS uses a very dated bash by default (bash 3.2.57(1) as of macOS 14.1). That is likely part of the I see with attempting the installation script. I did temporarily update to bash 5 using Homebrew, but that did not qualitatively change the picture. The errors I saw are below. Having said that, I think it's fine to simply clarify in the instructions that macOS is not a supported OS.

prf % bash ./install_proffast_linux.sh
adapting source code ...
sed: 1: "source/invers/globinv24 ...": unterminated substitute in regular expression
sed: 1: "source/invers/invers24.f90": unterminated substitute pattern
sed: 1: "source/pcxs/globlev24.f90": unterminated substitute in regular expression
sed: 1: "source/pcxs/globlin24.f90": unterminated substitute in regular expression
sed: 1: "source/pcxs/globvar24.f90": unterminated substitute in regular expression
sed: 1: "source/pcxs/pcxs24.f90": unterminated substitute pattern
sed: 1: "source/preprocess/glob_ ...": bad flag in substitute command: 'b'
sed: 1: "source/preprocess/glob_ ...": bad flag in substitute command: 'b'
sed: 1: "source/preprocess/prepr ...": bad flag in substitute command: 'c'
compiling preprocess ...
ld: library not found for -lSystem
collect2: error: ld returned 1 exit status
compiling invers...
ld: library not found for -lSystem
collect2: error: ld returned 1 exit status
compiling pcxs...
ld: library not found for -lSystem
collect2: error: ld returned 1 exit status
restoring files ...
sed: 1: "source/invers/globinv24 ...": unterminated substitute in regular expression
sed: 1: "source/invers/invers24.f90": unterminated substitute pattern
sed: 1: "source/pcxs/globlev24.f90": unterminated substitute in regular expression
sed: 1: "source/pcxs/globlin24.f90": unterminated substitute in regular expression
sed: 1: "source/pcxs/globvar24.f90": unterminated substitute in regular expression
sed: 1: "source/pcxs/pcxs24.f90": unterminated substitute pattern
sed: 1: "source/preprocess/glob_ ...": bad flag in substitute command: 'b'
sed: 1: "source/preprocess/glob_ ...": bad flag in substitute command: 'b'
sed: 1: "source/preprocess/prepr ...": bad flag in substitute command: 'c'
done.
feldl commented 5 months ago

Dear @simonom and @usethedata, thank you very much for your helpul feedback. We updated the installation instructions as you suggested.

Specifically we 1) Added a "Prerequisites" section, where the Python version and operation systems are specified (stating that Mac is not supported) 2) Restructured the section with the instructions to install PROFFAST, and clarified to install gfortran for Linux, and to run the installation script from the prf folder 3) Removed the duplicate instruction to execute the example The changes are already uploaded to the documentation web page

Please let us know if any other improvements are required.

usethedata commented 5 months ago

I'm good with this.

simonom commented 5 months ago

My checklist is now complete and I recommend publication in JOSS. Well done to the authors.

rwegener2 commented 5 months ago

Great, thank you @usethedata and @simonom for your review!

rwegener2 commented 5 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

Editor checks paper proof:

Editor checks archive generated by author:

Editor double checks paper and recommends submission:

rwegener2 commented 5 months ago

@editorialbot generate pdf

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:

rwegener2 commented 5 months ago

@editorialbot check references

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

OK DOIs

- 10.5194/amt-8-3059-2015 is OK
- 10.5194/acp-19-3271-2019 is OK
- 10.5194/amt-12-1513-2019 is OK
- 10.5194/amt-13-4751-2020 is OK
- 10.3390/rs13173395 is OK
- 10.5194/amt-15-2199-2022 is OK
- 10.5194/amt-15-2433-2022 is OK
- 10.5194/acp-22-295-2022 is OK
- 10.5194/amt-9-2303-2016 is OK
- 10.5194/amt-14-1111-2021 is OK
- 10.21105/joss.05131 is OK
- 10.5194/amt-4-47-2011 is OK
- 10.5445/IR/1000159168 is OK
- 10.5194/amt-11-2173-2018 is OK
- 10.5194/amt-14-5887-2021 is OK
- 10.14291/TCCON.GGG2014.DOCUMENTATION.R0/1221662 is OK
- 10.5194/amt-13-4791-2020 is OK

MISSING DOIs

- No DOI given, and none found for title: COCCON Data Processing
- No DOI given, and none found for title: Using a portable FTIR spectrometer for checking th...

INVALID DOIs

- None
rwegener2 commented 5 months ago

Hi @feldl πŸ‘‹πŸ»

Can you please make the follow grammar corrections or clarifications in the paper text:

~cross through~ is the existing text to be deleted. Bolded text is text to be added.

rwegener2 commented 5 months ago

@feldl at this point could you please:

I can then move forward with recommending acceptance of the submission.

feldl commented 5 months ago

@rwegener2, thank you very much for your suggestions; we updated the manuscript accordingly.

I set a version tag to mark a new release and archived this version at Zenodo:

Version 1.3.2 10.5281/zenodo.11035671

feldl commented 5 months ago

@editorialbot generate pdf

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:

rwegener2 commented 4 months ago

@editorialbot set 10.5281/zenodo.11035671 as archive

editorialbot commented 4 months ago

Done! archive is now 10.5281/zenodo.11035671

rwegener2 commented 4 months ago

@editorialbot set 1.3.2 as version

editorialbot commented 4 months ago

Done! version is now 1.3.2

rwegener2 commented 4 months ago

@editorialbot set v1.3.2 as version

editorialbot commented 4 months ago

Done! version is now v1.3.2

rwegener2 commented 4 months ago

@editorialbot recommend-accept

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

OK DOIs

- 10.5194/amt-8-3059-2015 is OK
- 10.5194/acp-19-3271-2019 is OK
- 10.5194/amt-12-1513-2019 is OK
- 10.5194/amt-13-4751-2020 is OK
- 10.3390/rs13173395 is OK
- 10.5194/amt-15-2199-2022 is OK
- 10.5194/amt-15-2433-2022 is OK
- 10.5194/acp-22-295-2022 is OK
- 10.5194/amt-9-2303-2016 is OK
- 10.5194/amt-14-1111-2021 is OK
- 10.21105/joss.05131 is OK
- 10.5194/amt-4-47-2011 is OK
- 10.5445/IR/1000159168 is OK
- 10.5194/amt-11-2173-2018 is OK
- 10.5194/amt-14-5887-2021 is OK
- 10.14291/TCCON.GGG2014.DOCUMENTATION.R0/1221662 is OK
- 10.5194/amt-13-4791-2020 is OK

MISSING DOIs

- No DOI given, and none found for title: COCCON Data Processing
- No DOI given, and none found for title: Using a portable FTIR spectrometer for checking th...

INVALID DOIs

- None
editorialbot commented 4 months 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/5287, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

kthyng commented 4 months ago

Hi! I'll take over now as Track Associate Editor in Chief to do some final submission editing checks. After these checks are complete, I will publish your submission!

kthyng commented 4 months ago

Everything looks great!

kthyng commented 4 months ago

@editorialbot accept

editorialbot commented 4 months ago
Doing it live! Attempting automated processing of paper acceptance...
editorialbot commented 4 months ago

Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository.

If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file.

You can copy the contents for your CITATION.cff file here:

CITATION.cff

``` cff-version: "1.2.0" authors: - family-names: Feld given-names: Lena orcid: "https://orcid.org/0009-0000-9955-7658" - family-names: Herkommer given-names: Benedikt orcid: "https://orcid.org/0000-0001-5784-2127" - family-names: Vestner given-names: Jasmin - family-names: Dubravica given-names: Darko - family-names: Alberti given-names: Carlos orcid: "https://orcid.org/0000-0002-1574-5393" - family-names: Hase given-names: Frank contact: - family-names: Feld given-names: Lena orcid: "https://orcid.org/0009-0000-9955-7658" doi: 10.5281/zenodo.11035671 message: If you use this software, please cite our article in the Journal of Open Source Software. preferred-citation: authors: - family-names: Feld given-names: Lena orcid: "https://orcid.org/0009-0000-9955-7658" - family-names: Herkommer given-names: Benedikt orcid: "https://orcid.org/0000-0001-5784-2127" - family-names: Vestner given-names: Jasmin - family-names: Dubravica given-names: Darko - family-names: Alberti given-names: Carlos orcid: "https://orcid.org/0000-0002-1574-5393" - family-names: Hase given-names: Frank date-published: 2024-04-30 doi: 10.21105/joss.06481 issn: 2475-9066 issue: 96 journal: Journal of Open Source Software publisher: name: Open Journals start: 6481 title: "PROFFASTpylot: Running PROFFAST with Python" type: article url: "https://joss.theoj.org/papers/10.21105/joss.06481" volume: 9 title: "PROFFASTpylot: Running PROFFAST with Python" ```

If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation.

Find more information on .cff files here and here.

editorialbot commented 4 months ago

🐘🐘🐘 πŸ‘‰ Toot for this paper πŸ‘ˆ 🐘🐘🐘

editorialbot commented 4 months ago

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/5288
  2. Wait five minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.06481
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! πŸŽ‰πŸŒˆπŸ¦„πŸ’ƒπŸ‘»πŸ€˜

Any issues? Notify your editorial technical team...