Joss-reviews: [REVIEW]: Masks2Metrics (M2M): A Matlab Toolbox for Gold Standard Morphometrics

Created on 24 Oct 2017  ยท  51Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @smikhael (Shadia Mikhael)
Repository: https://github.com/Edinburgh-Imaging/Masks2Metrics
Version: v1.0
Editor: @cMadan
Reviewer: @Kevin-Mattheus-Moerman
Archive: 10.7488/ds/2302

Status

status

Status badge code:

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

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 questions

@Kevin-Mattheus-Moerman, please carry out your review in this issue by updating the checklist below (please make sure you're logged in to GitHub). The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @cMadan know.

Conflict of interest

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.0)?
  • [x] Authorship: Has the submitting author (@smikhael) 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)?
accepted published recommend-accept review

Most helpful comment

@cMadan I've ticked all the boxes. I recommend that we accept this submission. ๐Ÿ‘

All 51 comments

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @Kevin-Mattheus-Moerman 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 as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS 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

@smikhael I've started the review process. I've a couple of points before I dive any deeper.

I opened an issue in relation to contributing guidelines and one about help/documentation and adding a "getting started" section to the readme.md.

The former issue is a recommendation, however the latter I feel is really required since (as a new user) I'm unsure where to start.

Is there a DOI for the Bauer_2013 reference? Perhaps remove the self citation in the paper (Mikhael_2017) to the github link of this project, or replace it by reference/DOI to an archive (e.g. ZENODO) once an initial release is created.

That's excellent. Thank you @cMadan and @Kevin-Mattheus-Moerman for agreeing to be the editor and reviewer.
So far I have (1) added a doi and url for the Bauer_2013 reference (and will replace the self citation by a doi once an initial release is created, as suggested), and (2) added a 'getting started' section to the home page as well as links to the tutorial and workflow on both that page and the wiki home page, just so the user doesn't miss them. It is definitely off-putting when they don't see/have them.
I will next look into the contributing guidelines you recommended.
Thank you for looking into this project so promptly.

Hello all- @cMadan and @Kevin-Mattheus-Moerman! ๐Ÿ‘‹ Wondering whether there are any other pending issues you'd like me to address, or is that everything done? I can see that the project is still 'under review', but as it's my first submission to JOSS I'm not sure whether there's anything that I need to do at this point in time. Please advise. Many thanks.

Hi @smikhael I'm still working on the review I should be able to finish it this week. If there are more issues I'll create them on your repo and mention them here.

Hi @Kevin-Mattheus-Moerman thanks for the clarification. I'll keep an eye out on both pages.

@Kevin-Mattheus-Moerman, any update? Thanks!

@Kevin-Mattheus-Moerman @cMadan we are planning for January to release some evaluations/comparisons of metrics against other software - is there anything else we should do for the repo/article? thx

Right now we're just waiting on @Kevin-Mattheus-Moerman to finish the review/give more feedback. Nothing else needed from your side at this time.

@Kevin-Mattheus-Moerman, nudge?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #436 with the following error: 

 /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/author.rb:36:in `block in build_affiliation_string': Problem with affiliations for Calum Gray, perhaps the affiliations index need quoting? (RuntimeError)
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/author.rb:35:in `each'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/author.rb:35:in `build_affiliation_string'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/author.rb:11:in `initialize'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon.rb:97:in `new'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon.rb:97:in `block in parse_authors'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon.rb:95:in `each'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon.rb:95:in `parse_authors'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon.rb:76:in `initialize'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/processor.rb:26:in `new'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/processor.rb:26:in `set_paper'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/bin/whedon:37:in `prepare'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in `run'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in `invoke_command'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in `dispatch'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in `start'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/bin/whedon:99:in `<top (required)>'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `load'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `<main>'

@cMadan @CPernet @smikhael
I've completed the review and could verify that your software does _what it says on the can_. I have some minor comments below.

About the wiki

The Wiki home link starts with

Masks2Metrics (M2M) is a quick and robust Matlab-based tool that calculates 3 metrics for a given region-of-interest (ROI)- its thickness, its volume, and the surface area of the inner mask.

There may be different definitions of what an ROI is, e.g. ROI may be masks or are sometimes defined as closed polygon sets. I think in your case they are binary pixels sets that are continuous lines for each slice in a particular direction? Perhaps define what you mean by ROI. Could you also provide a link to the NIfTI file format definition or a link to tools that are able to open/create them?

I think you should probably remove the word function in the wiki (under home and usage), i.e. you would not run it with function like that as a command you probably accidentally pasted it in:

After browsing to the directory containing the paired masks, the code is run using a single command line of the form:
function [roi_gm_mean_thickness, roi_gm_vol, roi_wm_sa] = masks2metrics(subj, segments, roi, hem, step_size, draw)

About the example

Not sure if this is serious / an issue only on my machine, but upon running the tutorial in the wiki I get this warning (I use MATLAB 2016b, Ubuntu 17.10):

Warning: Could not start Excel server for export.
XLSWRITE will attempt to write file in CSV format.
In xlswrite (line 174)
In masks2metrics (line 209)

The example does not say where the files are saved. I found the file 1_sfg_l_mean_frechet_step3.mat in the current working directory. Could the user set the location? Why are these files saved separately like that? I also see csv files. Could you add a description of all data that is saved, where it is saved and what they represent, in the wiki? What can the user do with these files?

The following is not a requirement but a recommendation. The example rapidly generates ~27 figures. The images in these figures are very small and difficult to interpret. Also as a first example having 27 tiny figures splash across the screen seems a bit overwhelming/difficult to understand. The figures in the actual wiki (like figure 1 under home and figure 1 and 2 in the tutorial) are much clearer and contain a description. I.e. they are zoomed in on a feature and have a legend/description. Could you make the figures generated by the example very similar or actually have the example replicate these figures? Would you be able to create an anatomical view in matlab as well so the users can see what these masks represent? Also would it be possible to add legends to the figures to denote what the dots/symbols/lines mean? You could also explain in the wiki what the figures are going to show. So warn them about 27 figures E.g. by running this MATLAB will generate 27 figures representing each slice (?) and visualizing each mask and the distances metrics and areas between them?

About the readme

There is a minor typo / an awkward sentence in the readme you should rephrase, (I think a similar statement is well written in the paper so you could perhaps copy it over) it starts:

The ROI are defined by paired of masks...

About the paper

Can you check the above error message after attempting to create the pdf. It seems something is up with the affiliations. @arfon may be able to advise.

@CPernet @smikhael apologies for the delay in processing this review. I've left one lasts check mark open for when you reply to my comments above.

Happy holidays :christmas_tree: :sun_with_face:

@Kevin-Mattheus-Moerman , thank you for you that helpful feedback, and no worries regarding the delay. I'll address all points in the next few days and update you once done. Happy holidays to all!

Thank you for the detailed review, @Kevin-Mattheus-Moerman!

Along with the suggestions above, I'd suggest maybe having a flag to enable/disable the figure generation.

After you make these last few edits, I'll give the a code a quick try and see if I have any additional usability suggestions. I think I'm well within the target audience (and do plan to use this for a current project), so hopefully that'll be useful. After that we'll be all set!

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #436 with the following error: 

   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100    15    0    15    0     0     36      0 --:--:-- --:--:-- --:--:--    36
sh: 1: cd: can't cd to tmp/436/paper
Looks like we failed to compile the PDF

@Kevin-Mattheus-Moerman @cMadan thank you all for your helpful feedback once again. I have now attempted to address all the points you identified as follows:

  • flag to enable/disable figures: there was a flag (last variable draw when calling masks2metrics), but I've now made it much clearer using warnings in both the code and the Wiki. Apologies for all those figures jumping at you. They're quite annoying when undesired!

  • Changes to the Wiki:

    • correct, the word function was accidentally pasted in. I've now deleted it.
    • added extra line explaining the ROI, as suggested
    • added link to nifti file format
    • added reference to MRIcron as a tool for drawn nifti files and pointing to their drawing tutorial.
    • added table describing all files generated
    • added 'Additional Figure Information' section detailing the contents of each of the figures
    • added a warning to set the variable/flag 'draw' to 0 in case the user doesn't want figures
  • Changes to Masks2Metrics:

    • deleted the writing of the (thickness, volume and surface area) measurements to .xls files (previously lines 209-214). This is both confusing and redundant (used in the early days of the code only) as we are also writing these measurements to .mat files.
    • added a warning at the beginning (lines 74-84) which only appears if the value of the flag 'draw' passed to the function equals 1. If draw=0 then the figures are not generated. The warning reminds the user that figures will be generated for every segment, asks them to quit if they don't want that, and to call Masks2Metrics with flag draw=0. The note also points the user to the wiki for more information on figure content.
    • got it to print out total thickness parameters, as well as surface area and volume of the ROI, and to save (.mat format) those that weren't being saved previously.
  • Readme:

    • typo now fixed. New text says: The ROI is defined by pairs of 3-dimensional (3D) binary masks (in NIfTI format) that represent the inner and outer borders of the ROI,...
  • pdf generation:

    • Lastly, I've attempted to fix the affiliations problem of the 2nd author (added " " around 1,2 for multiple affiliations) but I still can't generate a pdf. I believe it can't 'cd' to the paper. @arfon any advice please?

Please let me know if you'd like me to address any additional issues. I look forward to your feedback.

I hope this proves useful to your work @cMadan and to that of many others! Let me know if you run into any issues. I'm happy to help.

Thank you all once again.

Just a quick thought re: the PDF compiling, I think the issue is that there are spaces in "paper for JOSS". Can you try renaming the folder to just "paper" or replacing the spaces with underscores?

I will look over the rest of your responses (and try the toolbox) in the next few days.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #436 with the following error: 

 /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in `parse': (tmp/436/JOSS_paper/paper.md): invalid leading UTF-8 octet at line 1 column 1 (Psych::SyntaxError)
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in `parse_stream'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:327:in `parse'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:254:in `load'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:475:in `block in load_file'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in `open'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in `load_file'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon.rb:73:in `initialize'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/processor.rb:26:in `new'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/lib/whedon/processor.rb:26:in `set_paper'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/bin/whedon:37:in `prepare'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in `run'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in `invoke_command'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in `dispatch'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in `start'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-4bc3ad988bea/bin/whedon:99:in `<top (required)>'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `load'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `<main>'

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
https://github.com/openjournals/joss-papers/blob/joss.00436/joss.00436/10.21105.joss.00436.pdf

Pdf success! Thank you for the tip @cMadan

It looks like your references heading does not render well. Perhaps a space is needed

References

Thank you @Kevin-Mattheus-Moerman . I've now added the space, and fixed 2 other referencing issues that I discovered (1. needed '_' between Kochunov and 2012, and 2. added the Mills_2016 in the .bib file.). Apologies for the oversight. Please let me know if you'd like me to regenerate the pdf.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
https://github.com/openjournals/joss-papers/blob/joss.00436/joss.00436/10.21105.joss.00436.pdf

The paper looks good

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...
https://github.com/openjournals/joss-papers/blob/joss.00436/joss.00436/10.21105.joss.00436.pdf

Apologies, but I've just regenerated pdf as I uncovered a mistake in one reference (the GitHub reference to our code). The way I had written it made it appear as though the reference only had one author (me) rather than 2. The pdf should now be typo-free. Thank you.

Hello all, @cMadan @Kevin-Mattheus-Moerman . I was wondering whether you had any updates for us on this please? Thank you.

Sorry for the delay, I will finalize my pass of the code by the end of the week.

Great, many thanks!

@cMadan I've ticked all the boxes. I recommend that we accept this submission. ๐Ÿ‘

@smikhael, it looks like everything is in order! Can you archive the current code on Zenodo or FigShare and tell me the DOI?

Excellent, thank you! @cMadan , I archived it just last week on Edinburgh DataShare at https://datashare.is.ed.ac.uk/handle/10283/3018, with the doi: 10.7488/ds/2302. Is that any good or do you prefer another archive on Zenodo/FigShare?

@whedon set 10.7488/ds/2302 as archive

OK. 10.7488/ds/2302 is the archive.

@Kevin-Mattheus-Moerman - many thanks for your review here and to @cMadan for editing this submission โœจ

@smikhael - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00436 โšก๏ธ ๐Ÿš€ ๐Ÿ’ฅ

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

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

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:

Thank you all! I've now added a link to the paper from the README.

Was this page helpful?
0 / 5 - 0 ratings