openjournals / joss-reviews

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

[REVIEW]: MatSurv: Survival analysis and visualization in MATLAB #1830

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @jhcreed (Jordan Creed) Repository: https://github.com/aebergl/MatSurv Version: v1.1.0 Editor: @cMadan Reviewer: @dsurujon, @ManuelaS Archive: 10.5281/zenodo.3632122

Status

status

Status badge code:

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

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

@dsurujon & @ManuelaS, 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 @cMadan know.

Please try and complete your review in the next two weeks

Review checklist for @dsurujon

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @ManuelaS

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @dsurujon, @ManuelaS it looks like you're currently assigned to review 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:

  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
whedon commented 4 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 4 years ago

PDF failed to compile for issue #1830 with the following error:

/app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-1e4ee47b240d/lib/whedon.rb:135:in check_fields': Paper YAML header is missing expected fields: date (RuntimeError) from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-1e4ee47b240d/lib/whedon.rb:87:ininitialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-1e4ee47b240d/lib/whedon/processor.rb:36:in new' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-1e4ee47b240d/lib/whedon/processor.rb:36:inset_paper' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-1e4ee47b240d/bin/whedon:55:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-1e4ee47b240d/bin/whedon:116:in<top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

cMadan commented 4 years ago

@jhcreed, can you please fix your paper.md? It looks like it's missing the date information. Compare https://raw.githubusercontent.com/aebergl/MatSurv/master/Article/paper.md with https://raw.githubusercontent.com/adrn/gala/f6bb2532afeed3c0b0feea3037bf71a3f42b2a12/paper/paper.md

jhcreed commented 4 years ago

Sorry! I have added the date and double checked that it should compile.

cMadan commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 4 years ago

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

cMadan commented 4 years ago

@jhcreed, skimming your paper I found quite a few typos ("long", "calues", and a few others). Can you and your co-authors take a closer look at the text?

jhcreed commented 4 years ago

We have looked back over it and fixed the typos. Thanks!

cMadan commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago
Attempting PDF compilation. Reticulating splines etc...
cMadan commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 4 years ago

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

dsurujon commented 4 years ago

Thank you for this opportunity to review MatSurv. I have gone through the manuscript, documentation and example usage cases. I believe this is a very useful piece of code, and works well, with plenty of options for customization of the output figures. The authors have supplied a number of reproducible examples that are very helpful as well. However, I believe the authors can better highlight the utility of MatSurv by addressing the points I raise below. Once these are addressed, I will be happy to recommend the manuscript for publication.

General checks

Authorship
I see another user @pjl54 has made a commit, but doesn't appear as an author. Depending on the extent of their contribution, I suggest including them as an author or at least mentioning them in the acknowledgements if it was a minor contribution.

Documentation

Statement of need
Please include a description of what contexts survival analysis would be useful in, and what the target audience is.

Community guidelines
Please include this section in the documentation

Software paper

Statement of need In the summary section, I believe the authors can be more specific with some examples of when survival analysis would be applied, and what kinds of conclusions can be drawn from the output of MatSurv and similar software. I believe the Summary section could include more information on when and how survival analysis is used, since this might not be clear for a general audience.

Quality of writing

References

Other comments

Minor comments

cMadan commented 4 years ago

@dsurujon, thank you for the thorough review!

cMadan commented 4 years ago

@ManuelaS, it would be great if you could begin your review soon, thanks!

ManuelaS commented 4 years ago

Thanks for the reminder @cMadan, I'll work through the review during the weekend!

danielskatz commented 4 years ago

👋 @ManuelaS - how is your review going?

aebergl commented 4 years ago

Hello all, We have made the changes to MatSurv and the Documentation as suggested by @dsurujon and we are waiting for the second reviewer comments so we can finalize our edits of the article.

ManuelaS commented 4 years ago

Apologies for the delay. I'll get to this in the next few days.

ManuelaS commented 4 years ago

Review Apologies for the delay to getting to this review. I enjoyed reading through the manuscript and using MatSurv. MatSurv is well-written, easy to “install” and use, has good documentation and offers plenty of options to tailor Kaplan-Meier plots to the specific study needs. I believe MatSurv will be a useful addition for the MATLAB community.

Below some suggestions:

Minor comments:

aebergl commented 4 years ago

We would like to start with thanking @dsurujon for his excellent work and insightful comments. Below you will find our changes and response to the individual comments in bold below.

General checks

Authorship I see another user @pjl54 has made a commit, but doesn't appear as an author. Depending on the extent of their contribution, I suggest including them as an author or at least mentioning them in the acknowledgements if it was a minor contribution. We have now added Patrick Leo to the acknowledgements

Documentation

Statement of need Please include a description of what contexts survival analysis would be useful in, and what the target audience is. This have now been added

Community guidelines Please include this section in the documentation This is now included

Software paper

Statement of need In the summary section, I believe the authors can be more specific with some examples of when survival analysis would be applied, and what kinds of conclusions can be drawn from the output of MatSurv and similar software. I believe the Summary section could include more information on when and how survival analysis is used, since this might not be clear for a general audience. We have rewritten this section and also added more text

Quality of writing

  • I found the writing difficult to follow. The first half of the Summary section and the first paragraph of the Use section are mostly definitions, which could benefit a lot from being put into context, with descriptions of why these statistics are used, and how they give us meaningful information. More text have now been added
  • It is not clear which statistics MatSurv outputs in the text. It appears that the "stats" output includes hazard ratios and 95% CIs calculated both with the log-rank and the Mantel-Haenszel approach. However, the section under "Use" implies only the log-rank results are returned. This have now been changed
  • The authors make a point about how the log-rank test "will give slightly different results when compared to the Mantel-Haneszel or Cox regression approach, which is commonly used in R.", but then show agreement between their output and the R output in Table 1. This is confusing, but can be clarified by explaining what exactly is being reported in Table 1. This has now been clarified in the article
  • The chi square statistic in Table 1 is not defined, and as it stands, it is not clear how this is relevant to the log-rank test. Chi square has now been explained

References

  • For the mentioned survival analysis approaches (log rank test, hazard ration and Kaplan-Meier curves), it would be helpful to have references. More references has been added
  • The data sources (e.g. Freireich et al., 1963) should also be referenced. This reference have been added

Other comments

  • I highly recommend the authors explain some key concepts such as what an "event" and a "censored event" is. The risk table included in the output should also be introduced both in the paper and in the documentation. We have added text describing this in the paper and also in the documentation
  • In the risk table, it is not immediately clear that the columns correspond to the same time points as in the KM curve figure. I suggest adding another row (representing Time, as in ggsurvplot) as default. Great comment! We have now added this as a default option and also added an option to put the risk table as a part of the KM-plot
  • In the documentation, I suggest describing the datasets used in more detail, as it would add to why this software is important/useful We have now added text describing the data better
  • While the input options are defined in the documentation, there is little information on the output (especially the stats output variable). I suggest describing each individual output statistic similar to what's included as comments in the MatSurv.m script We have now added this section in the documentation
  • Table 1 does not have a legened, and it appears that in the "Data" column, the last row has a typo (LMAL instead of LAML) Fixed

Minor comments

aebergl commented 4 years ago

Thanks for the exellent review and great suggestions by @ManuelaS. Your comments have greatly improved the functionality of MatSurv.

Review Apologies for the delay to getting to this review. I enjoyed reading through the manuscript and using MatSurv. MatSurv is well-written, easy to “install” and use, has good documentation and offers plenty of options to tailor Kaplan-Meier plots to the specific study needs. I believe MatSurv will be a useful addition for the MATLAB community. Thanks!

Below some suggestions:

Minor comments:

Originally posted by @ManuelaS in https://github.com/openjournals/joss-reviews/issues/1830#issuecomment-564997236

aebergl commented 4 years ago

@cMadan We have addressed and commented on the changes that we have made to MatSurv. What should we do next? I'm a bit new to the process, so sorry if I have missed anything.

cMadan commented 4 years ago

@aebergl @jhcreed, thank you for your work on responding to the reviewers. At this stage the reviewers should look over your changes to the project and see if they are satisfied with your updates/responses.

@dsurujon @ManuelaS, it would be great if you could look over the changes the authors have made and let us know what you think!

dsurujon commented 4 years ago

It looks like the article text has been updated substantially. Could you please compile an updated article proof?

aebergl commented 4 years ago

@cMadan Hello again. Is generating the pdf something we can do?

cMadan commented 4 years ago

@whedon generate pdf

cMadan commented 4 years ago

@aebergl, the @whedon generate pdf is something that the authors are able to do themselves, as needed.

whedon commented 4 years ago

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

aebergl commented 4 years ago

@cMadan Thanks for you help and letting me know how to do this myself next time. The proofs looks good.

ManuelaS commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1182/blood.V21.6.699.699 is OK

MISSING DOIs

- https://doi.org/10.2307/2532873 may be missing for title: Survival analysis, a self‐learning text.
- https://doi.org/10.1136/bmjopen-2019-030215 may be missing for title: Proposals on Kaplan-Meier plots in medical research and a survey of stakeholder views: KMunicate

INVALID DOIs

- None
ManuelaS commented 4 years ago

@aebergl & @cMadan, the changes to the ReadMe and manuscript along with the addition of unit tests and bug fixes look good to me. Regarding the unit tests, I am guessing the "ground truth" stats included in the mat files that the test script checks against were generated by MatSurv. I wonder (no review-breaking) whether adding some tests that compare the output of MatSurv to the statistical results from an independent software (for example the survival package from R) would help in catching potential errors in the computations arising in corner cases.

P.S. It looks like a couple of references may be missing the DOI

I am happy to recommend acceptance!

jhcreed commented 4 years ago

@whedon check references

whedon commented 4 years ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@whedon commands
jhcreed commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1182/blood.V21.6.699.699 is OK

MISSING DOIs

- https://doi.org/10.2307/2532873 may be missing for title: Survival analysis, a self‐learning text.
- https://doi.org/10.1136/bmjopen-2019-030215 may be missing for title: Proposals on Kaplan-Meier plots in medical research and a survey of stakeholder views: KMunicate

INVALID DOIs

- None
cMadan commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1007/978-1-4419-6646-9 is OK
- 10.1136/bmjopen-2019-030215 is OK
- 10.1182/blood.V21.6.699.699 is OK

MISSING DOIs

- None

INVALID DOIs

- None
cMadan commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

aebergl commented 4 years ago

The proofs looks good. No edits from me

dsurujon commented 4 years ago

The changes on the manuscript, software and documentation look great. I am happy to recommend this updated version for publication!

aebergl commented 4 years ago

@cMadan Hello again. What is the next step? Cheers, /Anders

cMadan commented 4 years ago

@aebergl @jhcreed, everything looks good to me!

To move forward with accepting your submission, there are a few last things to take care of:

You may find this helpful: https://guides.github.com/activities/citable-code/

aebergl commented 4 years ago

@cMadan v1.1.0 DOI

cMadan commented 4 years ago

@whedon set v1.1.0 as version