Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: QMRTools: a mathematica toolbox for quantitative MRI analysis #1204

Closed
whedon opened this issue Jan 26, 2019 · 75 comments

Comments

Projects
None yet
7 participants
@whedon
Copy link
Collaborator

commented Jan 26, 2019

Submitting author: @mfroeling (Martijn Froeling)
Repository: https://github.com/mfroeling/QMRITools
Version: v2.2
Editor: @cMadan
Reviewers: @grlee77, @JoshKarpel, @rljacobson
Archive: 10.5281/zenodo.3239261

Status

status

Status badge code:

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

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

@grlee77 & @JoshKarpel, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @cMadan know.

Please try and complete your review in the next two weeks

Review checklist for @grlee77

Conflict of interest

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: v2.2
  • Authorship: Has the submitting author (@mfroeling) 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 @JoshKarpel

Conflict of interest

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: v2.2
  • Authorship: Has the submitting author (@mfroeling) 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 @rljacobson

Conflict of interest

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: v2.2
  • Authorship: Has the submitting author (@mfroeling) 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)?
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 26, 2019

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

⭐️ Important ⭐️

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
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 26, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 26, 2019

@cMadan

This comment has been minimized.

Copy link
Member

commented Jan 26, 2019

@whedon check references

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 26, 2019

Attempting to check references for missing DOIs
@cMadan

This comment has been minimized.

Copy link
Member

commented Jan 27, 2019

@whedon add @rljacobson as reviewer

@whedon whedon assigned cMadan, grlee77 and JoshKarpel and unassigned cMadan Jan 27, 2019

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 27, 2019

OK, @rljacobson is now a reviewer

@JoshKarpel

This comment has been minimized.

Copy link

commented Jan 31, 2019

(Adding this here instead of as an issue because it's about the paper, not the software itself)

I think the paper could be improved by breaking out the list of included toolboxes into a full bullet list and adding a short description to each, like there is in the README. It wouldn't dramatically increase the length of the paper, and would help people identify what specific functionality the package can help them with.

That could also feed into some more specifics about the target audience and how this helps them. What kind of people would benefit from using this package over whatever other tools are available?

@JoshKarpel

This comment has been minimized.

Copy link

commented Jan 31, 2019

Calling @grlee77 and @rljacobson to mfroeling/QMRITools#6 when you get a chance - any ideas for automated testing?

@mfroeling

This comment has been minimized.

Copy link

commented Jan 31, 2019

@JoshKarpel i will make the paper a bit more explanatory with regard to the points you raised.

@grlee77

This comment has been minimized.

Copy link
Collaborator

commented Feb 6, 2019

For reference, the following issues were opened at the source repo:
@JoshKarpel opened an issue related to testing: mfroeling/QMRITools#6
@grlee77 opened an issue related to documentation: mfroeling/QMRITools#7

@grlee77

This comment has been minimized.

Copy link
Collaborator

commented Feb 7, 2019

What kind of people would benefit from using this package over whatever other tools are available?

It appears that engineers and MR physicists as opposed to clinicians/radiologists are the target audience. Fat/water fraction estimation, diffusion tensor estimation and T2-fitting based on extended phase graphs (EPG) are the three primary applications that are highlighted. All of these are of current interest to researchers in the field of MRI.

There are a number of other widely used software packages for diffusion tensor analysis across a range of programming languages. I am not sure if there are any others implemented in Mathematica, though. I think there is much less current open source software for Dixon fat/water estimation and for EPG-based T2-fitting. Would you agree with this @mfroeling or is there some other unique capability in regards to diffusion imaging (perhaps the parameter estimate vs. SNR simulations?)

@grlee77

This comment has been minimized.

Copy link
Collaborator

commented Feb 7, 2019

One other potential issue that should probably be pointed out in the documentation:

Although the software has a permissive BSD 3-clause license, in practice use in commercial applications may be prohibited due to existing patents (I'm pretty sure this is the case for the IDEAL fat/water estimation algorithm?). Each MR system vendor tends to sell their own proprietary closed-source software options for fat/water estimation.

@mfroeling

This comment has been minimized.

Copy link

commented Feb 7, 2019

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 7, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 7, 2019

@mfroeling

This comment has been minimized.

Copy link

commented Feb 7, 2019

@JoshKarpel I have updated the paper with a small paragraph of each toolbox and added a few lines how the toolbox came about.

@grlee77 @JoshKarpel most indeed my toolbox does what most DTI frameworks can and by now there are many, when i started 10 years ago as a student working on DTI of muscle most of them didnt exist and mathematica was my preferred language. As such i started coding my own scripts and functions and this toolbox is the result of that. I'm sure that there are toolboxes out there that are better and more refined regarding some aspects. For my personal need (mostly muscle and cardiac) there arent any tools that allowed me to perform the tasks i wanted. for dixon there is the matlab based ISMRM dixon toolbox and for EPG there are non as far as i am aware of.

@grlee77 yes IDEAL is patented, so is Dixon, and so is DTI. Do you think however all methods are openly published and can be used. I'm not sure what the consequences are for people that want to use this in commercial settings and if that is something i should care about. it is the one that want to commertially exploit this who has to take care of that or am i mistaken. would a ohter license be more suited?

@grlee77

This comment has been minimized.

Copy link
Collaborator

commented Feb 8, 2019

@grlee77 yes IDEAL is patented, so is Dixon, and so is DTI. Do you think however all methods are openly published and can be used. I'm not sure what the consequences are for people that want to use this in commercial settings and if that is something i should care about. it is the one that want to commertially exploit this who has to take care of that or am i mistaken. would a ohter license be more suited?

I am not an IP/patent law expert, but I don't think you need to change the license of your code. I was just suggesting that it might be worth having a disclaimer warning users that some of the implemented methods may be covered under patent law. An example of this sort of disclaimer from the OpenCV project can be seen here:
https://github.com/opencv/opencv_contrib/blob/d6895a1b25d46af8f820faefe729a325629d0af1/modules/surface_matching/PATENTS.txt#L12-L29

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019

OK. v2.2 is the version.

@cMadan

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

@whedon set 10.5281/zenodo.3239261 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019

OK. 10.5281/zenodo.3239261 is the archive.

@cMadan

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019

Check final proof 👉 openjournals/joss-papers#737

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

@whedon accept deposit=true
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 6, 2019


OK DOIs

- 10.1002/jmri.23608 is OK
- 10.1002/jmri.24619 is OK
- 10.1002/nbm.3459 is OK
- 10.3389/fninf.2013.00050 is OK
- 10.1364/AO.41.007437 is OK
- 10.1002/mrm.26259 is OK
- 10.1002/mrm.26124 is OK
- 10.1186/1532-429X-17-S1-P15 is OK
- 10.1109/TMI.2009.2035616 is OK
- 10.1002/mrm.26059 is OK
- 10.1002/nbm.3427 is OK
- 10.1002/nbm.2959 is OK
- 10.1002/mrm.20624 is OK
- 10.1109/TMI.2008.920609 is OK
- 10.1016/j.neuroimage.2016.08.016 is OK
- 10.1109/ISBI.2006.1624856 is OK
- 10.1364/AO.46.006623 is OK
- 10.1002/mrm.21737 is OK
- 10.1002/(SICI)1522-2594(199909)42:3<515::AID-MRM14>3.0.CO;2-Q is OK
- 10.1002/mrm.24649 is OK
- 10.1016/j.jmr.2010.12.008 is OK
- 10.1002/mrm.24340 is OK
- 10.1016/j.neuroimage.2013.05.028 is OK
- 10.1002/mrm.25165 is OK
- 10.1161/hc0402.102975 is OK
- 10.1148/radiol.14140702 is OK

MISSING DOIs

- None

INVALID DOIs

- None
@cMadan

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

@openjournals/joss-eics, I think we're all set to accept here!

@labarba

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

@mfroeling Could you edit the Zenodo archive metadata so that the title matches the JOSS paper? Thanks!

@mfroeling

This comment has been minimized.

Copy link

commented Jun 7, 2019

@labarba The Zenodo tiltle now matches the JOSS paper.

@labarba

This comment has been minimized.

Copy link
Member

commented Jun 9, 2019

Some minor editorial suggestions:

  • ¶2: the acronym DWI is used for the first time, without defining (we don't know what that means). Also, recommend to delete "mostly" in first sentence. Add commas after "e.g."
  • ¶3: "some demo data which is" >> that is. Comma after "notebook."
  • "the documentation is build" >> built
  • "such that is incorporated" >> ?
  • Cardiac tools: "which allows defining the hard" >> ? (also, two "which allows" in a row)
  • "The first is based on and LMMSE framework" >> delete "and"? should you spell this out?
  • Check all other acronyms: can you spell these out on first use?
@mfroeling

This comment has been minimized.

Copy link

commented Jun 11, 2019

@labarba will make the suggested changes shortly.

@mfroeling

This comment has been minimized.

Copy link

commented Jun 11, 2019

@labarba The suggested changes are done.

@labarba labarba added the accepted label Jun 11, 2019

@labarba

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019

Check final proof 👉 openjournals/joss-papers#745

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

@whedon accept deposit=true
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019


OK DOIs

- 10.1002/jmri.23608 is OK
- 10.1002/jmri.24619 is OK
- 10.1002/nbm.3459 is OK
- 10.3389/fninf.2013.00050 is OK
- 10.1364/AO.41.007437 is OK
- 10.1002/mrm.26259 is OK
- 10.1002/mrm.26124 is OK
- 10.1186/1532-429X-17-S1-P15 is OK
- 10.1109/TMI.2009.2035616 is OK
- 10.1002/mrm.26059 is OK
- 10.1002/nbm.3427 is OK
- 10.1002/nbm.2959 is OK
- 10.1002/mrm.20624 is OK
- 10.1109/TMI.2008.920609 is OK
- 10.1016/j.neuroimage.2016.08.016 is OK
- 10.1109/ISBI.2006.1624856 is OK
- 10.1364/AO.46.006623 is OK
- 10.1002/mrm.21737 is OK
- 10.1002/(SICI)1522-2594(199909)42:3<515::AID-MRM14>3.0.CO;2-Q is OK
- 10.1002/mrm.24649 is OK
- 10.1016/j.jmr.2010.12.008 is OK
- 10.1002/mrm.24340 is OK
- 10.1016/j.neuroimage.2013.05.028 is OK
- 10.1002/mrm.25165 is OK
- 10.1161/hc0402.102975 is OK
- 10.1148/radiol.14140702 is OK

MISSING DOIs

- None

INVALID DOIs

- None
@labarba

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

@whedon accept deposit=true

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019

Doing it live! Attempting automated processing of paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 openjournals/joss-papers#746
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01204
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? notify your editorial technical team...

@labarba

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Congratulations, @mfroeling, your JOSS paper is published! 🚀

Big thanks to our editor: @cMadan, and reviewers: @grlee77, @JoshKarpel, @rljacobson — we are grateful for your contribution to JOSS 🙏

@labarba labarba closed this Jun 11, 2019

@whedon

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 11, 2019

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

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.01204/status.svg)](https://doi.org/10.21105/joss.01204)

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

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

This is how it will look in your documentation:

DOI

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:

@mfroeling

This comment has been minimized.

Copy link

commented Jun 12, 2019

@cMadan @grlee77 @JoshKarpel @rljacobson Thank you all for the review process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.