openjournals / joss-reviews

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

[REVIEW]: spant: An R package for magnetic resonance spectroscopy analysis #3646

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @martin3141 (Martin Wilson) Repository: https://github.com/martin3141/spant Version: v1.15.0 Editor: @chartgerink Reviewers: @joaomcteixeira, @chartgerink Archive: 10.5281/zenodo.5592186

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

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

@joaomcteixeira, 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 @chartgerink 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 @joaomcteixeira

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @chartgerink

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Note from the Editor: Due to a reviewer dropping out of the process, I did some aspects of the review - to verify some of the points raised during the process. This is primarily related to reading the paper for comprehensiveness, checking the repository, and installing the software (as issues were raised in that regard).

Conflict of interest

Code of Conduct

General checks

Functionality

Note: Functionality and performance were not checked as these were not raised as issues in the peer review.

Documentation

Note: Other aspects were not raised as potential issues in the review and were not checked.

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @a3sha2, @joaomcteixeira 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
whedon commented 3 years ago

Wordcount for paper.md is 542

whedon commented 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.31 s (1239.1 files/s, 283765.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                           302          14582           4169          46695
R                               44           2529           3539           9222
Fortran 77                       2             47           1186           1362
XML                              1              0              0            825
CSS                              4             99             49            431
Markdown                         4             65              0            357
JavaScript                       7             69             45            307
TeX                              1             22              0            214
Rmd                              8            119            314            182
YAML                             3             17              4             97
SVG                              1              0              1             11
C                                1              2              1              7
-------------------------------------------------------------------------------
SUM:                           378          17551           9308          59710
-------------------------------------------------------------------------------

Statistical information for the repository '03402958971278a309bd1535' was
gathered on 2021/08/23.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Martin Wilson                   16           888            181          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Martin Wilson               707           79.6         12.4               13.01
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5281/zenodo.5168794 is OK

MISSING DOIs

- 10.1002/mrm.21081 may be a valid DOI for title: An algorithm for the automated quantitation of metabolites in in-vitro NMR signals
- 10.1002/mrm.22579 may be a valid DOI for title: A constrained least-squares approach to the automated quantitation of in vivo ¹H magnetic resonance spectroscopy data
- 10.1002/nbm.4257 may be a valid DOI for title: Preprocessing, analysis and quantification in single-voxel magnetic resonance spectroscopy: experts’ consensus recommendations
- 10.1002/mrm.1910300604 may be a valid DOI for title: Estimation of metabolite concentrations from localized in vivo proton NMR spectra
- 10.1002/jmri.24478 may be a valid DOI for title: Gannet: A batch-processing tool for the quantitative analysis of gamma-aminobutyric acid–edited MR spectroscopy spectra
- 10.1101/2020.06.16.155291 may be a valid DOI for title: FSL-MRS: An end-to-end spectroscopy analysis package
- 10.1016/j.jneumeth.2020.108827 may be a valid DOI for title: Osprey: Open-source processing, reconstruction & estimation of magnetic resonance spectroscopy data
- 10.1016/s0010-4825(01)00006-3 may be a valid DOI for title: Java-based graphical user interface for MRUI, a software package for quantitation of in vivo/medical magnetic resonance spectroscopy signals
- 10.1002/nbm.1112 may be a valid DOI for title: An automated quantitation of short echo time MRS spectra in an open source software environment: AQSES
- 10.1002/mrm.28385 may be a valid DOI for title: Adaptive baseline fitting for ¹H MR spectroscopy analysis
- 10.1002/mrm.27605 may be a valid DOI for title: Robust retrospective frequency and phase correction for single-voxel MR spectroscopy
- 10.3390/cancers13102417 may be a valid DOI for title: Mapping of Metabolic Heterogeneity of Glioma Using MR-Spectroscopy
- 10.1038/s41398-020-00927-x may be a valid DOI for title: Exercise as a protective mechanism against the negative effects of oxidative stress in first-episode psychosis: a biomarker-led study
- 10.18637/jss.v044.i06 may be a valid DOI for title: Working with the DICOM and NIfTI Data Standards in R

INVALID DOIs

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

whedon commented 3 years ago

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

whedon commented 3 years ago

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

joaomcteixeira commented 3 years ago

Sorry for the delay, past week was impossible for me. I will try to review during this week. Sorry, it's September, lots of deadlines.

a3sha2 commented 3 years ago

@whedon I have started review and will be done this week. sorry for delay

whedon commented 3 years ago

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

@whedon commands
joaomcteixeira commented 3 years ago

First of all, congratulations on this solid and very promising software package.

I have gone through the paper, the repository, and the documentation. I am highlighting here some issues and suggestions I believe can improve the project and should be addressed before accepting the paper.

The article reads clearly. The English level is good, and there are no faults. The repository looks well organized. Also, the documentation pages are easy to navigate and contain useful information, and the API seems well documented.

I found some issues that could be corrected to clarify the project purposes and improve users and developers engagement with the software.

I am running Ubuntu 20.04 and following the installation instructions. I do not use R myself, so I had to install R from scratch. R and R-Studio installed well. I then followed the instructions to install spant running: install.packages("spant", dependencies = TRUE) . About 30 minutes have already passed, and R is still compiling packages (I have a powerful laptop). Is it supposed to take so long? If so, this should be stated as a warning to avoid the user's frustration. I will complete my review once the installation finishes.

Best,

martin3141 commented 3 years ago

Thank you @joaomcteixeira for taking the time to review this work and your encouraging and helpful comments. Please find my responses below:

It is not clear from the paper nor the documentation if spant can process raw FID data or if it works only on preprocessed data, that is, chemical shift vs intensity spectra. This should be clearly stated in the paper and the documentation.

"Raw data import/export." was a bulleted item in the Overview section. This has been expanded to emphasise the point. The paper has been updated to explain that raw data can be imported.

the paper states spant can process "large and complex datasets". What are large and complex datasets? A short explanation on the nature of these datasets and how they differ from "normal" datasets should be given.

Text has been added to emphasize the multidimensional nature of modern neuroimaging studies incorporating MRS.

It would be beneficial for the paper if it states that spant is already available on the MRSHub webpage. This gives value to spant because it shows the community has accepted it already.

The paper has been amended to include this suggestion.

In the documentation page, the tab "Articles" could read "Tutorials" instead.

I agree this is clearer and have made the suggested change.

it is not clear the version scheme adopted by the project. Is it SemanticVersion 2.0?

The format is “major.minor.patch.dev”, so yes I believe this is SemanticVersion 2.0.

there are no community guidelines on how to contribute to the project. For example, how to execute tests locally at the developer level, nor explanation on the tests will run at the repository level upon PR. Or how the CI, if it exists, is managed. Usually, this is stated in the CONTRIBUTING file.

A CONTRIBUTING.md file had been added to the repository as suggested.

martin3141 commented 3 years ago

Hi @chartgerink, just wondering what remains to be done here. I've responded to comments from @joaomcteixeira and am still waiting on a review from @a3sha2. Presumably my recent changes need to be approved, and we need a second completed review before the paper can be accepted?

joaomcteixeira commented 3 years ago

Hi @martin3141 , thanks for addressing my points. The only thing remaining from me is to try to install the package. Sorry I am really overwhelmed now. I will try to address this asap. Apart from that. Congratulations on the big work :-)

FYI, be mindful with the rules of SemanticVersioning 2.0, they are very useful when followed: https://semver.org/

chartgerink commented 3 years ago

Hi @martin3141 - I am awaiting @a3sha2 their review in order to continue the process. We set around four weeks for the reviews to be completed so this is all within the expected timeframe.

@a3sha2 will you be able to complete the review in the next week? 😊

martin3141 commented 3 years ago

Hi @martin3141 , thanks for addressing my points. The only thing remaining from me is to try to install the package. Sorry I am really overwhelmed now. I will try to address this asap. Apart from that. Congratulations on the big work :-)

FYI, be mindful with the rules of SemanticVersioning 2.0, they are very useful when followed: https://semver.org/

@joaomcteixeira thanks for letting me know. Installation on Windows is probably the easiest (no need to wait for any packages to compile or install any libraries) if you have a spare machine.

chartgerink commented 3 years ago

Hi @joaomcteixeira and @a3sha2 - would you be able to complete your reviews by next week? I would much appreciate rounding out your reviews so I can move to the next step and let @martin3141 know the next steps.

@a3sha2 I haven't heard from you since you accepted the review - please get in touch if there are external circumstances that prevent you from completing the review. Otherwise, I will have to continue without your review.

joaomcteixeira commented 3 years ago

Yes. I aim trying to find a easy to install protocol for Ubuntu as I cannot follow the instructions in the current manual (see https://github.com/martin3141/spant/issues/9). After that, I truly support going forward with the publication.

chartgerink commented 3 years ago

Thanks @joaomcteixeira for the update 😊

joaomcteixeira commented 3 years ago

@chartgerink

I have proposed @martin3141 installations instructions for Anaconda. Read here: https://github.com/martin3141/spant/issues/9

I think @martin3141 can add those instructions to the package. However, for me it is not a problem if Martin decides not to add them. This should not hinder publication, however it would considerably enhance the dissemination of the project. Answering the question: "Does the package installs correctly?" @martin3141 said he could install it on Ubuntu, but I couldn't. I can only install it and use it with the Anaconda-based approach. With the Anaconda env it seems to work nicely. I executed a couple of the official examples in a Jupyter Notebook and everything went accordingly.

Unless there is any other pressing issue you want me to address, I conclude here my revision of the spant package, and I support its publication.

Congratulations @martin3141 for your nice work :tada:

chartgerink commented 3 years ago

@whedon remove @a3sha2 as reviewer.

I have not heard from the reviewer since September 6th - I repeatedly inquired here and over email.

whedon commented 3 years ago

OK, @a3sha2 is no longer a reviewer

chartgerink 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.5281/zenodo.5168794 is OK

MISSING DOIs

- 10.1002/mrm.21081 may be a valid DOI for title: An algorithm for the automated quantitation of metabolites in in-vitro NMR signals
- 10.1002/mrm.22579 may be a valid DOI for title: A constrained least-squares approach to the automated quantitation of in vivo ¹H magnetic resonance spectroscopy data
- 10.1002/nbm.4257 may be a valid DOI for title: Preprocessing, analysis and quantification in single-voxel magnetic resonance spectroscopy: experts’ consensus recommendations
- 10.1002/mrm.1910300604 may be a valid DOI for title: Estimation of metabolite concentrations from localized in vivo proton NMR spectra
- 10.1002/jmri.24478 may be a valid DOI for title: Gannet: A batch-processing tool for the quantitative analysis of gamma-aminobutyric acid–edited MR spectroscopy spectra
- 10.1101/2020.06.16.155291 may be a valid DOI for title: FSL-MRS: An end-to-end spectroscopy analysis package
- 10.1016/j.jneumeth.2020.108827 may be a valid DOI for title: Osprey: Open-source processing, reconstruction & estimation of magnetic resonance spectroscopy data
- 10.1016/s0010-4825(01)00006-3 may be a valid DOI for title: Java-based graphical user interface for MRUI, a software package for quantitation of in vivo/medical magnetic resonance spectroscopy signals
- 10.1002/nbm.1112 may be a valid DOI for title: An automated quantitation of short echo time MRS spectra in an open source software environment: AQSES
- 10.1002/mrm.28385 may be a valid DOI for title: Adaptive baseline fitting for ¹H MR spectroscopy analysis
- 10.1002/mrm.27605 may be a valid DOI for title: Robust retrospective frequency and phase correction for single-voxel MR spectroscopy
- 10.3390/cancers13102417 may be a valid DOI for title: Mapping of Metabolic Heterogeneity of Glioma Using MR-Spectroscopy
- 10.1038/s41398-020-00927-x may be a valid DOI for title: Exercise as a protective mechanism against the negative effects of oxidative stress in first-episode psychosis: a biomarker-led study
- 10.18637/jss.v044.i06 may be a valid DOI for title: Working with the DICOM and NIfTI Data Standards in R

INVALID DOIs

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

chartgerink commented 3 years ago

Dear @martin3141 👋

Thanks for bearing with us on this review process of your package. It has taken a bit longer than I had hoped - and I am also getting the hang of this as a first-time editor.

The paper presents an important package for R environments, which up to now has been more readily available in Python environments.

Based on the review, I wanted to ensure the installation worked on Linux platforms; I attempted the installation (https://github.com/martin3141/spant/issues/9) on a new Ubuntu instance with the new instructions. This did not work (FWIW, the error was ERROR: dependency ‘RNiftyReg’ is not available for package ‘spant’ and trying to install that led to the error ERROR: compilation failed for package ‘RcppEigen’).

As the installation is a remaining issue, this means I cannot yet accept the paper. I know you cannot control the user's machine, but there needs to be a clear way to install the software on Linux and its relevant dependencies. Please test on an empty install and verify the instructions are complete.

Below I outline the todo's for your revisions from my end. If you can resolve these, I am happy to accept the paper for publication.

Todo

joaomcteixeira commented 3 years ago

@martin3141

Try to find a way to make installation instructions using Anaconda, as we discussed in the issue. Only one or two the dependencies were not installable. Almost everybody uses Anaconda right now. Those who don't, won't care if you ask them to install Anaconda, and those who do, will actually appreciate you have instructions for Anaconda. You can also try a folder-contained Miniconda installation, such that the Anaconda doesn't propagate to the whole system. Good luck!

martin3141 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.1002/mrm.21081 is OK
- 10.1002/mrm.22579 is OK
- 10.1002/nbm.4257 is OK
- 10.1002/mrm.1910300604 is OK
- 10.1002/jmri.24478 is OK
- 10.1002/mrm.28630 is OK
- 10.1016/j.jneumeth.2020.108827 is OK
- 10.1016/s0010-4825(01)00006-3 is OK
- 10.1002/nbm.1112 is OK
- 10.1002/mrm.27742 is OK
- 10.1148/radiol.13130531 is OK
- 10.1002/mrm.28385 is OK
- 10.1002/mrm.27605 is OK
- 10.3390/cancers13102417 is OK
- 10.1038/s41398-020-00927-x is OK
- 10.18637/jss.v044.i06 is OK
- 10.5281/zenodo.5168794 is OK

MISSING DOIs

- None

INVALID DOIs

- None
martin3141 commented 3 years ago

Dear @chartgerink,

Thanks for your final checks and suggestions.

1) References fixed. 2) I wasn't able to reproduce your problem, but the install commands are slightly different for Ubuntu 21.10, so perhaps this is the cause. I've added instructions for that specific version to the main github page. 3) Checks fixed. Hopefully you can see from : https://github.com/martin3141/spant/actions/runs/1349259672 that the software installs and passes internal and standard CRAN checks on the most recent LTS versions of Ubuntu (18.04 and 20.04).

@joaomcteixeira thanks for the tips on Anaconda, I will definitely try this 👍

Martin

martin3141 commented 3 years ago

@martin3141

Try to find a way to make installation instructions using Anaconda, as we discussed in the issue. Only one or two the dependencies were not installable. Almost everybody uses Anaconda right now. Those who don't, won't care if you ask them to install Anaconda, and those who do, will actually appreciate you have instructions for Anaconda. You can also try a folder-contained Miniconda installation, such that the Anaconda doesn't propagate to the whole system. Good luck!

Thanks @joaomcteixeira, your instructions worked for me and have been added to the README.md.

chartgerink commented 3 years ago

Hi @martin3141 👋 Thanks for being so responsive, and my apologies for the delay on my part.

I am happy take the recommendation by @joaomcteixeira and accept the paper 🥳 You handled all concerns I had, and thank you for your patience in this process.

There is one final step for you: Please make a tagged release and archive of the repository (e.g., Zenodo, figShare), and report the version number and archive DOI in this thread.

Once you let me know the archive DOI and version number, I will add them to the paper and pass it along to the Editor in Chief (see here for the procedural details).

martin3141 commented 3 years ago

Hi @chartgerink,

Zenodo DOI: 10.5281/zenodo.5592186 Version number: 1.15.0

Big thanks to you and @joaomcteixeira for agreeing to be editor / reviewer for this submission.

Cheers!

Martin

joaomcteixeira commented 3 years ago

Congratulations on your software and paper! :tada:

chartgerink commented 3 years ago

@whedon set 10.5281/zenodo.5592186 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.5592186 is the archive.

chartgerink commented 3 years ago

@whedon recommend-accept

whedon commented 3 years ago
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1002/mrm.21081 is OK
- 10.1002/mrm.22579 is OK
- 10.1002/nbm.4257 is OK
- 10.1002/mrm.1910300604 is OK
- 10.1002/jmri.24478 is OK
- 10.1002/mrm.28630 is OK
- 10.1016/j.jneumeth.2020.108827 is OK
- 10.1016/s0010-4825(01)00006-3 is OK
- 10.1002/nbm.1112 is OK
- 10.1002/mrm.27742 is OK
- 10.1148/radiol.13130531 is OK
- 10.1002/mrm.28385 is OK
- 10.1002/mrm.27605 is OK
- 10.3390/cancers13102417 is OK
- 10.1038/s41398-020-00927-x is OK
- 10.18637/jss.v044.i06 is OK
- 10.5281/zenodo.5168794 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2708

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/2708, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
chartgerink commented 3 years ago

I added a review checklist for myself to the initial comment, to document that I (editor of the paper) handled some issues from the review by @joaomcteixeira (e.g., installation). This helps openly document the process here, as it is a hybrid model due to a reviewer dropping out of the process.

Thanks @kthyng for helping me learn about the editor process at JOSS, as I am still a new editor here 😊

martin3141 commented 3 years ago

Hi @chartgerink @kthyng I'm wondering what remains to be done before the paper can be accepted?

kthyng commented 3 years ago

Hi @martin3141 — after one reviewer ghosted, @chartgerink was gracious enough to take over as reviewer on top of editing duties so as to avoid a long delay in your submission review. We have been discussing a little behind the scenes to make sure everything is covered given this. It looks like we can move forward now.

kthyng commented 3 years ago

@whedon add @chartgerink as reviewer

whedon commented 3 years ago

OK, @chartgerink is now a reviewer

kthyng commented 3 years ago

@whedon set v1.15.0 as version

whedon commented 3 years ago

OK. v1.15.0 is the version.

kthyng commented 3 years ago

@martin3141 Please update the Zenodo archive metadata so the title and author list exactly match your JOSS submission.

kthyng commented 3 years ago

@martin3141 Please check through your references in detail to catch capitalization mistakes. For example, Clayden et al should have capital R and C++. You can preserve capitalization by placing {} around characters/strings in your bib file.