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]: flux-data-qaqc: A Python Package for Energy Balance Closure and Post-Processing of Eddy Flux Data #3418

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @JohnVolk (John Volk) Repository: https://github.com/Open-ET/flux-data-qaqc Version: 0.1.6 Editor: @pdebuyl Reviewer: @ashwinvis, @dgketchum Archive: 10.5281/zenodo.5573104

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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

@ashwinvis & @dgketchum, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

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

Review checklist for @ashwinvis

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @dgketchum

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ashwinvis, @dgketchum it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
pdebuyl commented 3 years ago

@ashwinvis @dgketchum make sure to accept the invitation to the reviewers group and to have a look at the reviewer guidelines linked to at the top of this review page.

The review process will happen in this issue page, so questions to the author or to me can be added as comments here. As this is the first JOSS review for both reviewers (unless I missed something), do not hesitate to ask questions if you have doubts about the procedure. You can also take a look at earlier reviews to get an idea of how the reviews proceed https://github.com/openjournals/joss-reviews/issues?q=is%3Aissue+label%3Aaccepted+

@JohnVolk both reviewers gave indication that they can't start the review immediately. Given the remaining disruption because of the pandemic, JOSS operates with less strict schedules than usual. As there are also more vacations during the summer, I am already sorry for the delays.

whedon commented 3 years ago

:wave: @ashwinvis, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @dgketchum, please update us on how your review is going (this is an automated reminder).

pdebuyl commented 3 years ago

@ashwinvis @dgketchum any timeline for the review?

ashwinvis commented 3 years ago

@pdebuyl I will be back from my leave on September.

dgketchum commented 3 years ago

I'm ready to complete the checklist but it is not editable, and the link to the invitation is dead. @pdebuyl, advice?

danielskatz commented 3 years ago

@whedon re-invite @dgketchum as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

@dgketchum please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

dgketchum commented 3 years ago

@pdebuyl, this is a well-written paper describing a very useful software package. All checklist items cleared my review except one: automated testing.

pdebuyl commented 3 years ago

Hi @dgketchum thank you for the review!

pdebuyl commented 3 years ago

@JohnVolk keep in mind that @ashwinvis is on leave and will come back in september.

pdebuyl commented 3 years ago

@JohnVolk you may already address the automated testing of course.

JohnVolk commented 3 years ago

I've implemented automated testing using a GitHub actions workflow see here and included a badge in the README. I'll continue to add more tests to cover most of the package functionality. Thanks!

pdebuyl commented 3 years ago

Thank you @JohnVolk for the update.

pdebuyl commented 3 years ago

@ashwinvis do you have a timeline for the review by now?

ashwinvis commented 3 years ago

@pdebuyl Yes. Give me some time till 3rd October.

pdebuyl commented 3 years ago

thank you for the update @ashwinvis

pdebuyl commented 3 years ago

@JohnVolk can you describe shortly the test in the docs? There is one test file, if I understand well, that runs one typical setup and checks a few programmatic details (existence of output files) and some quantitative tests on the numerical output.

ashwinvis commented 3 years ago

@whedon check references

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5194/bg-15-5015-2018 is OK
- 10.1061/9780784408056 is OK
- 10.1175/1520-0477(2001)082<2415:FANTTS>2.3.CO;2 is OK
- 10.5281/zenodo.3831489 is OK
- 10.1038/s41597-020-0534-3 is OK

MISSING DOIs

- 10.2307/1941631 may be a valid DOI for title: Measuring biosphere-atmosphere exchanges of biologically related gases with micrometeorological methods

INVALID DOIs

- https://doi.org/10.1016/j.agrformet.2011.12.002 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1890/06-0922.1 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1002/joc.3413 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/S0168-1923(02)00109-0 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/j.agrformet.2012.11.004 is INVALID because of 'https://doi.org/' prefix
ashwinvis commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

JohnVolk commented 3 years ago

@ashwinvis I just updated the DOIs to remove those prefixes as recommended.

@pdebuyl I'll get add documentation for the testing soon. You are correct, the tests create a few different runs of the software using different example input files which are provided with the software, and checks if calculations and output files were performed/generated correctly in each case.

dgketchum commented 3 years ago

I successfully ran the package tests. Thank you for your attention to this, @JohnVolk.

ashwinvis commented 3 years ago

Minor revisions needed

<!DOCTYPE html>

pdebuyl commented 3 years ago

Thank you @ashwinvis for the review.

I see that @JohnVolk started to reply in the issues, I'll check them but it is easier for me if the resolutions of the issues are mentioned here.

JohnVolk commented 3 years ago

Thank you @ashwinvis for the useful comments and suggestions. I've addressed most of the points related to the software paper in the last commit except at this point I cannot find another option for the Allen et al., 2005 reference. Still working on the documentation and automated test coverage. Regarding the Allen et al., 2005 report, it is specifically written around the equation and is the most appropriate citation. However I am reaching out the author to see if there are any other options, he is also a coauthor on this software paper.

JohnVolk commented 3 years ago

@ashwinvis, just FYI, I did a google search for the report "The ASCE Standardized Reference Evapotranspiration Equation" and the second link that comes up for me is a PDF version of the report.

JohnVolk commented 3 years ago

Also, just in case you are interested, I just heard back from Richard Allen, he mentioned that that report is fine to use as well as this one on his website. They are both final drafts of the main report, the appendices are also linked on the website. However, when I run the JOSS preprint with a URL and a DOI included in a citation, the reference style it applies does not show the URL in the bibliography, so I suppose we can drop the DOI for Allen at al. 2005 in favor of a link to the PDF in the bibliography.

pdebuyl commented 3 years ago

Hi @JohnVolk indeed, DOIs take precedence. Could you try a bibtex "note" field along

also available at URL

?

This would let readers find a freely accessible version.

ashwinvis commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

ashwinvis commented 3 years ago

@JohnVolk I noticed that some of the key methods utilize a package called refet (https://github.com/WSWUP/RefET). I think this should be acknowledged. Also note somewhere in the software paper what quantities are calculated using refet.

JohnVolk commented 3 years ago

Good point, refet is used for the ASCE reference ET and potential solar radiation calculations. I'll go ahead and add it into the Acknowledgements in the documentation and in the paper directly where the ASCE equation is mentioned.

By the way, @pdebuyl I added the note field for the URL link with the free document (here) and nothing was printed after running the preprint, either the syntax is off or bibtex options are set to not use it?

pdebuyl commented 3 years ago

@JohnVolk I'll check this citation issue. I tested it locally and it seems that we should edit our style file, which requires discussion in the JOSS team. I'll let you know when I have more info.

JohnVolk commented 3 years ago

Excellent!

JohnVolk commented 3 years ago

@JohnVolk I noticed that some of the key methods utilize a package called refet (https://github.com/WSWUP/RefET). I think this should be acknowledged. Also note somewhere in the software paper what quantities are calculated using refet.

This has been addressed.

ashwinvis commented 3 years ago

@JohnVolk Things that remain as far as I can see in the software paper:

  1. Define the acronym QA/QC and what it means in this setting
  2. Clarify who the target audience is. It is fairly clear in the documentation, but not in the paper. If I was a reader I would wonder who the end-user would be.
  3. Fix this citation Ardö,:

    Pastorello, G., Trotta, C., Canfora, E., Chu, H., Christianson, D., Cheah, Y.-W., Poindexter,158 C., Chen, J., Elbashandy, A., Humphrey, M., Isaac, P., Polidori, D., Ribeca, A., Ingen, C.159 van, Zhang, L., Amiro, B., Ammann, C., Arain, M. A., Ardö, J., ... Papale, D. (2020).

JohnVolk commented 3 years ago

@ashwinvis I made some changes to the statement of need section to try to make the target audience clear, defined QA/QC, and fixed the citation. Thanks again for your careful review.

ashwinvis commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

ashwinvis commented 3 years ago

It is a bit of nit-picking, but I think the citations still needs some cleanup. Would it be proof-read after the review process @pdebuyl? In any case please go through all of them carefully, @JohnVolk

pdebuyl commented 3 years ago

Hi @ashwinvis I re-read the text and references as part of the editorial work after acceptance by the reviewers. If that is all that is holding you, don't worry about it :-)

@ashwinvis , @dgketchum , when the review is complete on your side, I'd like an explicit statement of acceptance here, it eliminates any ambiguity about the status of the paper.

ashwinvis commented 3 years ago

Thanks @pdebuyl, that's a relief. @JohnVolk, thank you for your patience. The code appears to be well documented and properly organized. This software paper would be a nice complement to it.

@pdebuyl, I recommend that the paper can be accepted.

JohnVolk commented 3 years ago

Thank you @ashwinvis, I just updated those all caps titles in the bibliography. Not sure about incorrect names in the Pastorello et al. (2020), there are special characters that the bibtex doesn't handle well but those names do not show up in the preprint, but I might be missing something.

pdebuyl commented 3 years ago

Thank you @ashwinvis for the review!

JohnVolk commented 3 years ago

Thank you @ashwinvis and @dgketchum for your reviews, and thank you @pdebuyl for everything you have done. This was a good experience.

dgketchum commented 3 years ago

@pdebuyl, I advise publication of this paper. Thank you!

pdebuyl commented 3 years ago

Thank you for the confirmation @dgketchum !