search

openjournals / joss-reviews

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

[REVIEW]: gsMAMS: an R package for Desinging Multi-Arm Multi-Stage Clinical Trials #6322

Closed editorialbot closed 5 months ago

editorialbot commented 9 months ago

Submitting author: !--author-handle-->@Tpatni719<!--end-author-handle-- (Tushar Patni) Repository: https://github.com/Tpatni719/gsMAMS.git Branch with paper.md (empty if default branch): Version: v0.7.2 Editor: !--editor-->@ppxasjsm<!--end-editor-- Reviewers: @njtierney, @RhysPeploe Archive: 10.5281/zenodo.11296405

Status

status

Status badge code:

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

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

@njtierney & @RhysPeploe, 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 @ppxasjsm 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 @RhysPeploe

📝 Checklist for @njtierney

editorialbot commented 9 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 9 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.18637/jss.v088.i04 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 9 months ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.04 s (478.2 files/s, 91167.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                               14            738            346           1774
HTML                             1             84              5            519
Markdown                         2             29              0             92
YAML                             1              1              4             18
TeX                              1              2              0             10
-------------------------------------------------------------------------------
SUM:                            19            854            355           2413
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 9 months ago

Wordcount for paper.md is 1467

editorialbot commented 9 months ago

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

ppxasjsm commented 8 months ago

Hi @njtierney, @RhysPeploe,

Please let me know if there is anything you need from me to get started with the review process. Thanks!

RhysPeploe commented 8 months ago

Hi @ppxasjsm, will be aiming to start work on the review later this week, expected completion by end of next week

RhysPeploe commented 8 months ago

Review checklist for @RhysPeploe

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

njtierney commented 8 months ago

Hi @ppxasjsm !

I'll be aiming to finish this review within the next 2 weeks. Cheers!

RhysPeploe commented 8 months ago

Hey,

I've notived a fair bit of commented (#) out code in the package, is this something JOSS will want cleaned up before accepting or is this okay? Looking at the guidelines 'JOSS requires that software should be feature-complete (i.e., no half-baked solutions), packaged appropriately according to common community standards for the programming language being used'

image

RhysPeploe commented 8 months ago

Hi all, Just to update on my checklist for Substanial Scholarly Effort - that depends on the comment above https://github.com/openjournals/joss-reviews/issues/6322#issuecomment-1997943810 @ppxasjsm thoughts on this? If you are happy with the commented code then I'll tick this off

And for Automated Tests, there does not appear to be any currently so when Issue #4 is resolved then that will be ticked off

Just those two issues, after which my checklist will be complete!

RhysPeploe commented 8 months ago

@ppxasjsm would you also be able to check issue #4 - automated tests and advise whether that satifies the chechlist list, please? update - not required, tests have been implemented

njtierney commented 7 months ago

Review checklist for @njtierney

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

njtierney commented 7 months ago

Regarding

Contribution and authorship: Has the submitting author (@Tpatni719) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

It is not clear to me how to evaluate the input of the author co-authors on this paper, there is no "contributions" or description of what other co-workers have done, and looking at the git contributions it seems like there are only commits from @Tpatni123 and @Tpatni719 who I think are the same author?

njtierney commented 7 months ago

Regarding

Does this submission meet the scope eligibility described in the JOSS guidelines

I note that the repository's first commit was Jan 10 - which makes it 3 months old currently but only 1 month old when submitted in February - however based on other metrics like lines of code it seems that this is a substantial piece of software. Measuring things is always hard and I do not like to reduce software to something like lines of code and age, but since they are in the guidelines I felt it was worth mentioning.

Whether the software is sufficiently useful that it is likely to be cited by your peer group.

I think that the paper.md requires some work to more clearly demonstrate the use case and audience for this work. I will write more substantial comments on this in another comment.

Tpatni719 commented 7 months ago

Regarding

Contribution and authorship: Has the submitting author (@Tpatni719) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

It is not clear to me how to evaluate the input of the author co-authors on this paper, there is no "contributions" or description of what other co-workers have done, and looking at the git contributions it seems like there are only commits from @Tpatni123 and @Tpatni719 who I think are the same author?

The other two authors had worked on the methodology code(the manuscript mentioned in the package at CRAN) and provided contributions in making the package user-friendly.

Tpatni719 commented 7 months ago

Regarding

Does this submission meet the scope eligibility described in the JOSS guidelines

I note that the repository's first commit was Jan 10 - which makes it 3 months old currently but only 1 month old when submitted in February - however based on other metrics like lines of code it seems that this is a substantial piece of software. Measuring things is always hard and I do not like to reduce software to something like lines of code and age, but since they are in the guidelines I felt it was worth mentioning.

The package was published at CRAN on 2023-05-04 and currently, it has around 6251 downloads.

njtierney commented 7 months ago

Hi @ppxasjsm I've actually got a few concerns about reproducibility and writing quality of the paper, which I have outlined in https://github.com/Tpatni719/gsMAMS/issues/17 - just wanted to flag that here.

njtierney commented 6 months ago

@ppxasjsm I confirm I have completed my review.

Tpatni719 commented 6 months ago

Hi @ppxasjsm, @njtierney, and @RhysPeploe have completed the review so please let me know the next steps.

RhysPeploe commented 6 months ago

Hi, I have one last query, refering back to the below comment regarding commented out sections of code in the functions - I personally would rather have the commented sections removed, however, if @ppxasjsm is happy the code satifies the guidelines then I am happy to tick off

Hey,

I've notived a fair bit of commented (#) out code in the package, is this something JOSS will want cleaned up before accepting or is this okay? Looking at the guidelines 'JOSS requires that software should be feature-complete (i.e., no half-baked solutions), packaged appropriately according to common community standards for the programming language being used'

image

Tpatni719 commented 6 months ago

Hi @RhysPeploe , I have removed the commented code from the scripts.

Tpatni123 commented 6 months ago

Hey,

I've noticed a fair bit of commented (#) out code in the package, is this something JOSS will want cleaned up before accepting or is this okay? Looking at the guidelines 'JOSS requires that software should be feature-complete (i.e., no half-baked solutions), packaged appropriately according to common community standards for the programming language being used'

Hi, @RhysPeploe, can you please check it and tick it off?

Kevin-Mattheus-Moerman commented 6 months ago

@Tpatni123 I'll contact the editor now to pick this up again. Apologies for the delay.

RhysPeploe commented 6 months ago

Hi,

Thanks for removing the commented sections, looks good apart from the below where there is a couple lines still in comments. There's two incidences where there are two lines of comments successively, where one max is needed. I understand some lines can help readability so I am happy for single lines of Hashtags to remain

I will complete my checklist now, on condition that the last remaining commented lines are resolved - thanks

Screenshot_20240508_121029_Samsung Internet

Screenshot_20240508_121259_Samsung Internet Screenshot_20240508_121207_Samsung Internet

Apologies for the screenshots I'm out of office and using my phone 😅

Kevin-Mattheus-Moerman commented 6 months ago

@Tpatni123 will you implement these requested changes?

ppxasjsm commented 6 months ago

My apologies for having dropped the ball on this, but somehow things got very hectic over the last couple of months and I had no capacity. I am just catching up with the thread. From what I see everything has been implemented? Looks like there is only one requested change open as far as I can tell. @Kevin-Mattheus-Moerman picked up on this already. If you could confirm this is done, I can move forward with this and we can get the paper accepted and published swiftly.

Tpatni719 commented 6 months ago

@ppxasjsm , I have done the required changes. Thanks!

ppxasjsm commented 6 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

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

ppxasjsm commented 6 months ago

@editorialbot check references

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

OK DOIs

- 10.18637/jss.v088.i04 is OK
- 10.1136/bmjopen-2015-009566 is OK
- 10.1177/009286159302700315 is OK

MISSING DOIs

- No DOI given, and none found for title: asd: Simulations for Adaptive Seamless   Designs
- No DOI given, and none found for title: AGSDest: Estimation in Adaptive Group Sequential T...
- No DOI given, and none found for title: adaptTest: Adaptive Two-Stage Tests
- 10.1007/978-3-642-01689-9 may be a valid DOI for title: Computation of Multivariate Normal and t Probabili...

INVALID DOIs

- https://doi.org/10.1016/S0140-6736(14)61122-3 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1002/sim.9682 is INVALID because of 'https://doi.org/' prefix
ppxasjsm commented 6 months ago

@Tpatni719 could you please check the references, add the missing ones and fix the DOIs where missing.

ppxasjsm commented 6 months ago

I have gone through the paper and there are two minor things you may consider addressing: https://github.com/Tpatni719/gsMAMS/issues/19

ppxasjsm commented 6 months ago

Once you have addressed the above, can you make a tagged release and archive, and report the version number and archive DOI in this thread? I can then set the archive version and we can proceed towards acceptance.

Tpatni719 commented 6 months ago

@Tpatni719 could you please check the references, add the missing ones and fix the DOIs where missing.

I have checked the invalid DOIs and they are working. Regarding the missing DOIs, unfortunately, they have not provided DOIs in their BibTex format.

Tpatni719 commented 6 months ago

Once you have addressed the above, can you make a tagged release and archive, and report the version number and archive DOI in this thread? I can then set the archive version and we can proceed towards acceptance.

Tag: v0.7.2 DOI: https://cran.r-project.org/web/packages/gsMAMS/index.html

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

ppxasjsm commented 6 months ago

@editorialbot check references

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

ppxasjsm commented 6 months ago

@editorialbot check references

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

OK DOIs

- 10.18637/jss.v088.i04 is OK
- 10.1136/bmjopen-2015-009566 is OK
- 10.1177/009286159302700315 is OK

MISSING DOIs

- Errored finding suggestions for "asd: Simulations for Adaptive Seamless   Designs", please try later
- No DOI given, and none found for title: AGSDest: Estimation in Adaptive Group Sequential T...
- No DOI given, and none found for title: adaptTest: Adaptive Two-Stage Tests
- 10.1007/978-3-642-01689-9 may be a valid DOI for title: Computation of Multivariate Normal and t Probabili...

INVALID DOIs

- https://doi.org/10.1016/S0140-6736(14)61122-3 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1002/sim.9682 is INVALID because of 'https://doi.org/' prefix
ppxasjsm commented 6 months ago

@editorialbot set v0.7.2 as version

editorialbot commented 6 months ago

Done! version is now v0.7.2

ppxasjsm commented 6 months ago

@Kevin-Mattheus-Moerman is the provided DOI an acceptable DOI? I am not super familiar with R-related things and does not seem like a standard registered DOI.

Also do you have any thoughts on the failed references check. To me they look like we can ignore them?

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

OK DOIs

- 10.18637/jss.v088.i04 is OK
- 10.1136/bmjopen-2015-009566 is OK
- 10.1177/009286159302700315 is OK

MISSING DOIs

- No DOI given, and none found for title: asd: Simulations for Adaptive Seamless   Designs
- No DOI given, and none found for title: AGSDest: Estimation in Adaptive Group Sequential T...
- No DOI given, and none found for title: adaptTest: Adaptive Two-Stage Tests
- 10.1007/978-3-642-01689-9 may be a valid DOI for title: Computation of Multivariate Normal and t Probabili...

INVALID DOIs

- https://doi.org/10.1016/S0140-6736(14)61122-3 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1002/sim.9682 is INVALID because of 'https://doi.org/' prefix
editorialbot commented 6 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.18637/jss.v088.i04 is OK
- 10.1016/S0140-6736(14)61122-3 is OK
- 10.1136/bmjopen-2015-009566 is OK
- 10.1177/009286159302700315 is OK
- 10.1002/sim.9682 is OK

MISSING DOIs

- No DOI given, and none found for title: asd: Simulations for Adaptive Seamless   Designs
- No DOI given, and none found for title: AGSDest: Estimation in Adaptive Group Sequential T...
- No DOI given, and none found for title: adaptTest: Adaptive Two-Stage Tests
- 10.1007/978-3-642-01689-9 may be a valid DOI for title: Computation of Multivariate Normal and t Probabili...

INVALID DOIs

- None