[REVIEW]: Estimating statistics from multi-state models using simulation with multistateutil

whedon commented 6 years ago

Submitting author: @stulacy (Stuart Lacy) Repository: https://github.com/stulacy/multistateutils Version: v1.2.0 Editor: @arfon Reviewer: @tgerke, @jenniferthompson, @rrrlw Archive: 10.5281/zenodo.1451474

Status

Status badge code:

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

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) 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

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

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

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @arfon know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @tgerke

Conflict of interest

[ ] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

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

General checks

[ ] Repository: Is the source code for this software available at the repository url?
[ ] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[ ] Version: Does the release version given match the GitHub release (v1.2.0)?
[ ] Authorship: Has the submitting author (@stulacy) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[ ] Installation: Does installation proceed as outlined in the documentation?
[ ] Functionality: Have the functional claims of the software been confirmed?
[ ] 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

[ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[ ] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[ ] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
[ ] 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

[ ] Authors: Does the paper.md file include a list of authors with their affiliations?
[ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[ ] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @jenniferthompson

Conflict of interest

[x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

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 repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Version: Does the release version given match the GitHub release (v1.2.0)?
[x] Authorship: Has the submitting author (@stulacy) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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 function of the software can be verified?
[ ] 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] Authors: Does the paper.md file include a list of authors with their affiliations?
[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] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @rrrlw

Conflict of interest

[x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

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 repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Version: Does the release version given match the GitHub release (v1.2.0)?
[x] Authorship: Has the submitting author (@stulacy) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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 function 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] Authors: Does the paper.md file include a list of authors with their affiliations?
[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] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

whedon commented 6 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @tgerke, it looks like you're currently assigned as the reviewer for this paper :tada:.

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

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

watching

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

whedon commented 6 years ago

Attempting PDF compilation. Reticulating splines etc...

whedon commented 6 years ago

--> Check article proof :page_facing_up: <--

rrrlw commented 6 years ago

Major suggestions have corresponding issues in associated GitHub repo:

Missing LICENSE plaintext file required for JOSS
Problem with R CMD build and undeclared dependency
Problem with R CMD check - not as certain about this one, can close issue if other reviewers don't run into the same problem

Minor suggestions (change only the ones you think will help future package users):

Had good examples for plot_predicted_pathway and predict_transitions; would be nice if there were corresponding examples in documentation for cohort_simulation, length_of_stay, and msprep2
Good practice to have package versions in Imports and Suggests fields of DESCRIPTION (e.g. dplyr (>=0.7)) to ensure that users with older versions of dependencies don't run into issues
?multistateutils::des-package and ?multistateutils::des` open template documentation, unsure if this should be filled in or omitted
Would be nice to add BugReports field in DESCRIPTION (probably link to GitHub repo Issues page)
Would be nice to add more detail in package description itself (?multistateutils), can probably use text from paper.md file in JOSS submission

Other notes:

devtools::install_github("stulacy/multistateutils") worked fine; examples in documentation ran fine
As best as I can tell, functions are working correctly, but I will let the reviewers with more expertise evaluate that
Created pull request to add DOI for a reference and add JOSS files to .Rbuildignore to prevent NOTE from R CMD check

arfon commented 6 years ago

@tgerke @jenniferthompson - how are you getting along with your reviews here?

jenniferthompson commented 6 years ago

I'm partway through, but ran into a stumbling block that I filed as an issue in the main repo. (Sorry, probably should have noted that here as well!) Unfortunately this laptop is the only machine I'll have access to for the next week, but I'll see if there's more I can add.

stulacy commented 6 years ago

Sorry I've been on holiday for the last few weeks. I'll have a look at that issue and the other suggestions above

arfon commented 6 years ago

:wave: @tgerke - when do you think you might be able to complete your review by?

stulacy commented 6 years ago

I've made the following changes and pushed to the development branch.

Major suggestions have corresponding issues in associated GitHub repo:

* [Missing LICENSE plaintext file required for JOSS](https://github.com/stulacy/multistateutils/issues/1)

Added LICENCE.md at directory top level

* [Problem with R CMD build and undeclared dependency](https://github.com/stulacy/multistateutils/issues/2)

Work in progress...

* [Problem with R CMD check](https://github.com/stulacy/multistateutils/issues/3) - not as certain about this one, can close issue if other reviewers don't run into the same problem

Work in progress...

Minor suggestions (change only the ones you think will help future package users):

* Had good examples for `plot_predicted_pathway` and `predict_transitions`; would be nice if there were corresponding examples in documentation for `cohort_simulation`, `length_of_stay`, and `msprep2`

Added examples for these functions.

* Good practice to have package versions in `Imports` and `Suggests` fields of DESCRIPTION (e.g. `dplyr (>=0.7)`) to ensure that users with older versions of dependencies don't run into issues

Haven't added yet although am open to doing so if it would be beneficial for users

* `?multistateutils::des-package` and ?multistateutils::des` open template documentation, unsure if this should be filled in or omitted

Removed these old leftover docs.

* Would be nice to add `BugReports` field in DESCRIPTION (probably link to GitHub repo Issues page)

Added.

* Would be nice to add more detail in package description itself (`?multistateutils`), can probably use text from paper.md file in JOSS submission

Decided not to add, would prefer users to read README or vignette.

rrrlw commented 6 years ago

Changes look great! Checked the corresponding boxes in this thread and responded to a couple things in repository issues page. Will recompile to ensure that updates to paper.bib made it properly to the article's PDF form, as compiled by @whedon.

rrrlw commented 6 years ago

@whedon generate pdf

whedon commented 6 years ago

Attempting PDF compilation. Reticulating splines etc...

whedon commented 6 years ago

:point_right: Check article proof :page_facing_up: :point_left:

rrrlw commented 6 years ago

With the closing of issues 1, 2, and 3, my review is complete.

Note that my review focused primarily on the mechanics of the multistateutils package (installation, documentation, etc.), and not on the content (I am not qualified for that). I have left one box unchecked in the reviewer checklist above; after @tgerke's and @jenniferthompson's reviews and corresponding code modifications, I would like to ensure that the package still installs correctly, and I will check the box once this has been confirmed.

Nicely done, @stulacy - looking forward to seeing multistateutils on JOSS!

arfon commented 6 years ago

:wave: @jenniferthompson - when do you think you might be able to complete your review by?

jenniferthompson commented 6 years ago

@arfon The fix @stulacy made recently means I can review on my laptop now, so I'll try to get back to it and have it done this weekend, if that works!

arfon commented 6 years ago

@arfon The fix @stulacy made recently means I can review on my laptop now, so I'll try to get back to it and have it done this weekend, if that works!

Sounds great. Thanks!

jenniferthompson commented 6 years ago

This package provides some highly useful extensions to existing capabilities in R for multistate models. Current capabilities are clearly highlighted in the Examples vignette, as well as the valuable additions provided by this package. The recently added examples are very helpful.

One issue I ran into is that the vignette doesn't seem to be building with the development version. I filed an issue in the main repo.

Minor suggestions

Agree with @rrrlw that adding more dependency info would be helpful; I often find it useful if I run into errors reproducing someone else's vignette code, for example. Might also be nice to explicitly note the dependencies on data.table and dplyr, as they're heavy dependencies that require some time to install.
Future documentation could include vignette(s) for cohort_simulations().
Currently no guidelines for new contributors that I could find; this could help guide others in contributing docs, etc - though I see @rrrlw checked this off, so possible I missed it?

Thank you for this helpful work!

rrrlw commented 6 years ago

@jenniferthompson, you are correct, on re-checking I failed to find contribution guidelines in the README or another file. Thank you for the correction!

Also removed text inappropriate from LICENSE file in this pull request.

stulacy commented 6 years ago

I've incorporated the LICENSE pull request and believe have fixed the vignette issue (see corresponding issue thread). I've also added a CONTRIBUTING.md file with guidelines on how to contribute, suggestions to improve it are welcome.

What is a good approach for adding package versions to Imports? I could use the current release version, but I don't want to force people to update to the latest version of say dplyr if they have reasons for keeping older versions and the older versions work anyway.

arfon commented 6 years ago

@stulacy - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

stulacy commented 6 years ago

I've made a new archive with the DOI 10.5281/zenodo.1451474

arfon commented 6 years ago

@whedon set 10.5281/zenodo.1451474 as archive

whedon commented 6 years ago

OK. 10.5281/zenodo.1451474 is the archive.

arfon commented 6 years ago

@jenniferthompson, @rrrlw - many thanks for your reviews here ✨

@stulacy - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00893 :zap: :rocket: :boom:

whedon commented 6 years ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.00893/status.svg)](https://doi.org/10.21105/joss.00893)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.00893">
  <img src="http://joss.theoj.org/papers/10.21105/joss.00893/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.00893/status.svg
   :target: https://doi.org/10.21105/joss.00893

This is how it will look in your documentation:

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html
Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=

stulacy commented 6 years ago

Thanks @arfon @jenniferthompson @rrrlw !

openjournals / joss-reviews