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]: morse: an R-package in support of Environmental Risk Assessment #3200

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: @sandrinecharles (Sandrine CHARLES) Repository: https://github.com/pveber/morse Version: v3.1.0 Editor: @KristinaRiemer Reviewers: @puruckertom, @Peter-Vermeiren Archive: 10.5281/zenodo.5771022

: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/1625fe55480a11b31989d7306f41e8f4"><img src="https://joss.theoj.org/papers/1625fe55480a11b31989d7306f41e8f4/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/1625fe55480a11b31989d7306f41e8f4/status.svg)](https://joss.theoj.org/papers/1625fe55480a11b31989d7306f41e8f4)

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

@BrandonEdwards & @puruckertom, 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 @KristinaRiemer 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 @puruckertom

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @Peter-Vermeiren

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

sandrinecharles commented 3 years ago

@KristinaRiemer all requests provided by @puruckertom were accounted for. The revised manuscript has been updated in the repository accordingly.

KristinaRiemer commented 2 years ago

@puruckertom have you had a chance to review these changes and update your checklist accordingly?

Also, @Peter-Vermeiren have you been able to start your review yet?

Peter-Vermeiren commented 2 years ago

@KristinaRiemer Thank you for your patience. I started my review, as planned in early November. It is my priority this week.

Peter-Vermeiren commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

KristinaRiemer commented 2 years ago

@whedon add @Peter-Vermeiren as reviewer

whedon commented 2 years ago

OK, @Peter-Vermeiren is now a reviewer

Peter-Vermeiren commented 2 years ago

Software paper -> Summary / Statement of need The summary mentions “full compliance with regulatory requirements”. I would have expected a bit of detail about that here, such as a reference or examples of relevant regulations, or the specific requirements you are referring to. I am not an expert on regulatory affairs. Would this package e.g. work for USA regulations as well as EU regulations?

Peter-Vermeiren commented 2 years ago

Software paper -> quality of writing

Submitted manuscript - version 09 Nov 2021.pdf

Peter-Vermeiren commented 2 years ago

Function survDataCheck: It would be nice to have a more informative message when everything works fine (currently: "no message"). How about: "data in correct format". Messages for other datatypes are informative, thanks. https://github.com/pveber/morse/blob/master/R/survDataCheck.R

Peter-Vermeiren commented 2 years ago

Help documentation for survFit function: The documentation seems to repeat itself 2 to 3 times. R/survFit.R https://github.com/pveber/morse/blob/master/R/survFit.R Minor spelling issue: L.9: The function \code{survFit} return -> returns with an “s”

Peter-Vermeiren commented 2 years ago

Thank you for the opportunity to review this manuscript and R package.

Peter-Vermeiren commented 2 years ago

p.s. I checked the relevant boxes in the review checklist.

KristinaRiemer commented 2 years ago

@sandrinecharles have you had a chance to review @Peter-Vermeiren's review yet?

And @puruckertom, I think you still need to look over the changes that have been made and update your review checklist?

sandrinecharles commented 2 years ago

Argh sorry @KristinaRiemer I totally forgot to do this... I will spend time by the end of the week for sure.

puruckertom commented 2 years ago

@KristinaRiemer @sandrinecharles I am happy with the revisions to my comments and have updated the checklist.

KristinaRiemer commented 2 years ago

@puruckertom great! Thanks for going over that.

@sandrinecharles let me know if you need anything during that process, and otherwise when you are finished.

sandrinecharles commented 2 years ago

@Peter-Vermeiren @KristinaRiemer

Software paper -> Summary / Statement of need The summary mentions “full compliance with regulatory requirements”. I would have expected a bit of detail about that here, such as a reference or examples of relevant regulations, or the specific requirements you are referring to. I am not an expert on regulatory affairs. Would this package e.g. work for USA regulations as well as EU regulations?

Fully agree with this suggestion; we added three references of different OECD guidance documents in our summary, and cited again the reference to EFSA PPR Panel 2018 within section "Survival analysis "(end of the first §). OECD references where of course added to the final list.

sandrinecharles commented 2 years ago

@Peter-Vermeiren @KristinaRiemer

Software paper -> quality of writing

  • There were some minor spelling errors in the paper, which I indicated with comments in the attached manuscript file.
  • I also added 3 points of clarification (a reference, clarification on what "advanced and innovative methods" are, and a clarification of a figure legend)

Submitted manuscript - version 09 Nov 2021.pdf

Thanks for pointing out spelling errors; we followed all your proposed changes, and also clarified where you suggested to do. Regarding spelling errors you pointed out in the message produced by 'morse' after the MFx calculations, I put an issue directly in the 'morse' package gitlab. This should be corrected with the next release but not changed immediately. I remove the grid in the background of Figure 5 (left panel).

sandrinecharles commented 2 years ago

@KristinaRiemer

Function survDataCheck: It would be nice to have a more informative message when everything works fine (currently: "no message"). How about: "data in correct format". Messages for other datatypes are informative, thanks. https://github.com/pveber/morse/blob/master/R/survDataCheck.R

Thanks @Peter-Vermeiren , nice suggestion that should also apply to the reproDataCheck function in 'morse'... I put another issue in our 'morse' gitlab for the next release. I asked that the next release is made asap, hopefully just before this manuscript is accepted, if it is finally.

sandrinecharles commented 2 years ago

@KristinaRiemer

Help documentation for survFit function: The documentation seems to repeat itself 2 to 3 times. R/survFit.R https://github.com/pveber/morse/blob/master/R/survFit.R Minor spelling issue: L.9: The function \code{survFit} return -> returns with an “s”

OK @Peter-Vermeiren , again an issue in the 'morse' gitlab for the next release. Thanks a lot for all your comments.

sandrinecharles commented 2 years ago

p.s. I checked the relevant boxes in the review checklist.

@Peter-Vermeiren you are right, automated test are not available.

sandrinecharles commented 2 years ago

Thank you for the opportunity to review this manuscript and R package.

  • I found the package very useful, with all functionalities that I would expect. The package is well documented so I could easily find how to adapt functions, as well as understand what the functions do and what their underlying theory is.
  • The paper is clear, with an easy-to-follow example. (It is nice that a more extensive tutorial is also available).
  • I only have very minor comments (see my previous 4 comments). The package and paper seems to have been tested and thought through quite extensively already.

@Peter-Vermeiren Thanks again for your helpful comments leading to improvement in our paper but also in the 'morse' package. @KristinaRiemer Thank you for having managed the review process. I achieved all changes related to comments from @Peter-Vermeiren. Accordingly, the gitlab dossier with paper files has been fully updated: https://github.com/pveber/morse/tree/master/paper.

Best regards, Sandrine

Peter-Vermeiren commented 2 years ago

@sandrinecharles Thank you for carefully considering each of my comments. @KristinaRiemer I am satisfied with the revisions. I think this package and paper will be interesting for both scientists and regulators. Best of luck, Peter

sandrinecharles commented 2 years ago

@KristinaRiemer According to reviewer's comments, we submitted the new release of our 'morse' package to the official CRAN web site two days ago.

KristinaRiemer commented 2 years ago

Great! Thank you so much @puruckertom & @Peter-Vermeiren for your reviews! Your work is greatly appreciated.

@sandrinecharles there a few more steps that we need to take care of before acceptance. First, we should both proofread the paper one last time to double check for typos, references, etc. Once that is satisfactory, you will make a release of and archive the code.

KristinaRiemer commented 2 years ago

@whedon generate pdf

KristinaRiemer commented 2 years ago

@whedon check references

whedon commented 2 years ago

Checking the BibTeX entries failed with the following error:

Failed to parse BibTeX on value "," (COMMA) [#, "@", #, {:author=>["OCDE"], :title=>["Daphnia magna Reproduction Test (OECD TG 211)"], :year=>["2018"]}, ",", "pages", "="]

whedon commented 2 years ago

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

KristinaRiemer commented 2 years ago

Checking the BibTeX entries failed with the following error:

Failed to parse BibTeX on value "," (COMMA) [#, "@", #, {:author=>["OCDE"], :title=>["Daphnia magna Reproduction Test (OECD TG 211)"], :year=>["2018"]}, ",", "pages", "="]

I think this whedon reference check error is occurring because the "Daphnia magna Reproduction Test" reference needs a number in the pages entry.

KristinaRiemer commented 2 years ago

These are the handful of typos/formatting that I was able to find in the paper:

I will wait to check the references for formatting, DOIs, etc. when the whedon error is resolved.

sandrinecharles commented 2 years ago

Checking the BibTeX entries failed with the following error: Failed to parse BibTeX on value "," (COMMA) [#, "@", #, {:author=>["OCDE"], :title=>["Daphnia magna Reproduction Test (OECD TG 211)"], :year=>["2018"]}, ",", "pages", "="]

I think this whedon reference check error is occurring because the "Daphnia magna Reproduction Test" reference needs a number in the pages entry.

OK I will check; this is probably due to the fact that this reference in a regulatory document not a standard article.

sandrinecharles commented 2 years ago

@KristinaRiemer The paper.bib file has been updated regarding OECD TG 211 (page number and year).

sandrinecharles commented 2 years ago
  • consistency in capitalization for TKTD full length (lines 29 & 37)

Caps are now removed on line 29, while the abbreviation TKTD is directly used on line 37.

  • "team in performing" to "team to perform"? (line 39-40)

done

  • "and" between EASYGUTS and GATEAUX in model list (line 41)

done

  • "can be either" to "can either be" (line 66)

done

  • "the Survival probability prediction error at the end of the exposure profile (SPPE)" to "the Survival Probability Prediction Error (SPPE) at the end of the exposure profile" or just capitalize "Survival Probability Prediction Error"? (lines 82-83)

Changes as "and the Survival Probability Prediction Error (SPPE) at the end of the exposure profile.".

Additionally, line 42, "frequestist" was changed to "frequentist".

The github repository of the paper has been updated accordingly.

KristinaRiemer commented 2 years ago

@whedon generate pdf

KristinaRiemer commented 2 years ago

@whedon check references

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

OK DOIs

- 10.1021/es502009r is OK
- 10.1787/9789264085275-en is OK
- 10.1787/9789264264335-en is OK
- 10.1787/9789264304741-12-en is OK
- 10.1007/s11356-017-9809-4 is OK
- 10.1021/acs.est.7b05464 is OK
- 10.1038/s41598-019-47698-0 is OK
- 10.2903/j.efsa.2018.5377 is OK
- 10.1002/ieam.4061 is OK
- 10.1007/s10646-012-0917-0 is OK

MISSING DOIs

- 10.1021/es801418x may be a valid DOI for title: A Bayesian Approach to Analyzing Ecotoxicological Data

INVALID DOIs

- None
whedon commented 2 years ago

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

KristinaRiemer commented 2 years ago

Thanks for making those edits to the text!

The references look pretty good, these were the only small errors I found:

sandrinecharles commented 2 years ago
  • replace link in ref for "A Bayesian Approach to Analyzing Ecotoxicological Data" with DOI (see whedon reference check above)

DOI added

  • that same reference also has you listed as author twice

corrected

  • the "Modelling survival under chemical stress" reference link returned that that page isn't found; is the ISBN number right?

I removed the isbn number as it does not appear ont the web page. The URL is correct: https://leanpub.com/guts_book.

  • For last three references, should "OCDE" be "OECD"?

Yes! changed.

The github repository of the paper has been updated accordingly just few minutes ago.

KristinaRiemer commented 2 years ago

@whedon check references

KristinaRiemer commented 2 years ago

@whedon generate pdf

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

OK DOIs

- 10.1021/es801418x is OK
- 10.1021/es502009r is OK
- 10.1787/9789264085275-en is OK
- 10.1787/9789264264335-en is OK
- 10.1787/9789264304741-12-en is OK
- 10.1007/s11356-017-9809-4 is OK
- 10.1021/acs.est.7b05464 is OK
- 10.1038/s41598-019-47698-0 is OK
- 10.2903/j.efsa.2018.5377 is OK
- 10.1002/ieam.4061 is OK
- 10.1007/s10646-012-0917-0 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 2 years ago

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

KristinaRiemer commented 2 years ago

Great, thanks for making all those changes! The paper looks good now to me.

The next steps are to: 1) create a tagged release on GitHub and 2) archive the code (on Zenodo, figshare, institutional repository, etc.) to generate a DOI. If you can post a link to the DOI here when you're done please.

sandrinecharles commented 2 years ago

Dear @KristinaRiemer, here is the DOI: 10.5281/zenodo.5771022 Best, Sandrine

KristinaRiemer commented 2 years ago

Hi @sandrinecharles, could you make the Zenodo archive title match the paper title?

sandrinecharles commented 2 years ago

Hi @sandrinecharles, could you make the Zenodo archive title match the paper title?

done :-)

KristinaRiemer commented 2 years ago

Perfect! Thank you.