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]: Bruker2nifti: Magnetic Resonance Images converter from Bruker ParaVision to Nifti format #354

Closed
whedon opened this Issue Aug 9, 2017 · 23 comments

Comments

Projects
None yet
6 participants
@whedon
Collaborator

whedon commented Aug 9, 2017

Submitting author: @SebastianoF (Sebastiano Ferraris)
Repository: https://github.com/SebastianoF/bruker2nifti
Version: 1.0.0
Editor: @Kevin-Mattheus-Moerman
Reviewer: @rougier
Archive: 10.5281/zenodo.846814

Status

status

Status badge code:

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

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

@rougier, 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 @Kevin-Mattheus-Moerman know.

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

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: Does the release version given match the GitHub release (1.0.0)?
  • Authorship: Has the submitting author (@SebastianoF) made major contributions to the software?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: Have any performance claims of the software been confirmed?

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.

Show comment
Hide comment
@whedon

whedon Aug 9, 2017

Collaborator

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @rougier 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 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
Collaborator

whedon commented Aug 9, 2017

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @rougier 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 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
@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment

👋 @SebastianoF @rougier @dzhoshkun @neurolabusc @effigies the review is happening here!

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment
@Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman Aug 9, 2017

Member

@rougier you are officially assigned and can tick the boxes at the top of this review issue. However others are invited to review the same aspects and leave comments here as well. Thanks!

@rougier you are officially assigned and can tick the boxes at the top of this review issue. However others are invited to review the same aspects and leave comments here as well. Thanks!

@rougier rougier self-assigned this Aug 10, 2017

@dzhoshkun

This comment has been minimized.

Show comment
Hide comment
@dzhoshkun

dzhoshkun Aug 10, 2017

@Kevin-Mattheus-Moerman Thanks! Looking forward to the feedback.

@Kevin-Mattheus-Moerman Thanks! Looking forward to the feedback.

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment
@whedon

This comment has been minimized.

Show comment
Hide comment
@whedon

whedon Aug 10, 2017

Collaborator

OK, the editor is @Kevin-Mattheus-Moerman

Collaborator

whedon commented Aug 10, 2017

OK, the editor is @Kevin-Mattheus-Moerman

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment

@whedon assign @rougier as reviewer

@whedon

This comment has been minimized.

Show comment
Hide comment
@whedon

whedon Aug 10, 2017

Collaborator

OK, the reviewer is @rougier

Collaborator

whedon commented Aug 10, 2017

OK, the reviewer is @rougier

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Aug 14, 2017

Collaborator

I've started the review and reported minor issues on the project repository. I think the submission is in good shape and I'm now waiting for author corrections before finishing the review.

Collaborator

rougier commented Aug 14, 2017

I've started the review and reported minor issues on the project repository. I think the submission is in good shape and I'm now waiting for author corrections before finishing the review.

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment

Thanks @rougier!

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Aug 14, 2017

Collaborator

I finished the review and @SebastianoF made all the changes, I think the submission can now be accepted and is ready for publication. See:

One remaining question is the possibility to update the submission with version 1.0.1 that is Python 3 compatible. I think it would be better to publish this version (which is also back compatible with Python 2.7).

Last very minor point is the PDF output missing from the source repo but JOSS will recompile it anyway, no?

Collaborator

rougier commented Aug 14, 2017

I finished the review and @SebastianoF made all the changes, I think the submission can now be accepted and is ready for publication. See:

One remaining question is the possibility to update the submission with version 1.0.1 that is Python 3 compatible. I think it would be better to publish this version (which is also back compatible with Python 2.7).

Last very minor point is the PDF output missing from the source repo but JOSS will recompile it anyway, no?

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment
@Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman Aug 21, 2017

Member

Thanks @rougier. Are we good to tick the last box? @arfon can you answer the PDF query

Last very minor point is the PDF output missing from the source repo but JOSS will recompile it anyway, no?

Thanks @rougier. Are we good to tick the last box? @arfon can you answer the PDF query

Last very minor point is the PDF output missing from the source repo but JOSS will recompile it anyway, no?

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment
@Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman Aug 21, 2017

Member

@SebastianoF I've some minor editorial comments on your paper below. Feel free to disagree.

I suggest you change

usually entails the image conversion from the native

to:

usually entails the conversion of image data from the native

I suggest you change:

The Bruker ParaVision proprietary software is not currently providing the tools for the data conversion to a suitable open formats for research, such as nifti [@cox2004sort], for which most of the available tools for medical image analysis are implemented.

to:

The proprietary Bruker ParaVision software currently does not provide the tools for conversion of the data to suitable and open formats for research, such as nifti [@cox2004sort], for which most of the available tools for medical image analysis are implemented.

In the following the sentence does not read well and and use of capital letters seems odd:

... and to parse every Parameter Files of the Bruker ParaVision format into python dictionaries...

Should this instead become?:

... and to parse all parameter files for the Bruker ParaVision format into python dictionaries...

Could the following sentence:

Lastly it is meant to be a starting point where to integrate possible future variations in Bruker hardware and ParaVision software future releases.

Be rephrased to something like:

Lastly it is meant to be a starting point to integrate possible future variations in Bruker hardware and ParaVision software.

@SebastianoF I've some minor editorial comments on your paper below. Feel free to disagree.

I suggest you change

usually entails the image conversion from the native

to:

usually entails the conversion of image data from the native

I suggest you change:

The Bruker ParaVision proprietary software is not currently providing the tools for the data conversion to a suitable open formats for research, such as nifti [@cox2004sort], for which most of the available tools for medical image analysis are implemented.

to:

The proprietary Bruker ParaVision software currently does not provide the tools for conversion of the data to suitable and open formats for research, such as nifti [@cox2004sort], for which most of the available tools for medical image analysis are implemented.

In the following the sentence does not read well and and use of capital letters seems odd:

... and to parse every Parameter Files of the Bruker ParaVision format into python dictionaries...

Should this instead become?:

... and to parse all parameter files for the Bruker ParaVision format into python dictionaries...

Could the following sentence:

Lastly it is meant to be a starting point where to integrate possible future variations in Bruker hardware and ParaVision software future releases.

Be rephrased to something like:

Lastly it is meant to be a starting point to integrate possible future variations in Bruker hardware and ParaVision software.

SebastianoF added a commit to SebastianoF/bruker2nifti that referenced this issue Aug 21, 2017

@SebastianoF

This comment has been minimized.

Show comment
Hide comment
@SebastianoF

SebastianoF Aug 21, 2017

@Kevin-Mattheus-Moerman thanks, it flows better now.

@Kevin-Mattheus-Moerman thanks, it flows better now.

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment
@Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman Aug 21, 2017

Member

🚀 Thanks. @SebastianoF and @rougier.

@arfon all boxes are ticked, and we are happy to accept this submission. Over to you.

🚀 Thanks. @SebastianoF and @rougier.

@arfon all boxes are ticked, and we are happy to accept this submission. Over to you.

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Aug 22, 2017

Member

Last very minor point is the PDF output missing from the source repo but JOSS will recompile it anyway, no?

Yep, I'll compile that shortly.

@SebastianoF - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

Member

arfon commented Aug 22, 2017

Last very minor point is the PDF output missing from the source repo but JOSS will recompile it anyway, no?

Yep, I'll compile that shortly.

@SebastianoF - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

@SebastianoF

This comment has been minimized.

Show comment
Hide comment

@arfon Hi, .zip uploaded to https://zenodo.org/record/846814.
The DOI is 10.5281/zenodo.846814
Thanks!

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Aug 27, 2017

Member

@whedon set 10.5281/zenodo.846814 as archive

Member

arfon commented Aug 27, 2017

@whedon set 10.5281/zenodo.846814 as archive

@whedon

This comment has been minimized.

Show comment
Hide comment
@whedon

whedon Aug 27, 2017

Collaborator

OK. 10.5281/zenodo.846814 is the archive.

Collaborator

whedon commented Aug 27, 2017

OK. 10.5281/zenodo.846814 is the archive.

@arfon arfon added the accepted label Aug 27, 2017

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Aug 27, 2017

Member

@rougier - many thanks for your review here and to @Kevin-Mattheus-Moerman for editing this submission

@SebastianoF - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00354 ⚡️ 🚀 💥

Member

arfon commented Aug 27, 2017

@rougier - many thanks for your review here and to @Kevin-Mattheus-Moerman for editing this submission

@SebastianoF - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00354 ⚡️ 🚀 💥

@arfon arfon closed this Aug 27, 2017

@SebastianoF

This comment has been minimized.

Show comment
Hide comment
@SebastianoF

SebastianoF Aug 27, 2017

@Kevin-Mattheus-Moerman @rougier @arfon 🚀 🚀 🚀 Thanks everyone! Best review-experience ever. Code have improved dramatically and I have learned a lot!
All the best

@Kevin-Mattheus-Moerman @rougier @arfon 🚀 🚀 🚀 Thanks everyone! Best review-experience ever. Code have improved dramatically and I have learned a lot!
All the best

@Kevin-Mattheus-Moerman

This comment has been minimized.

Show comment
Hide comment
@Kevin-Mattheus-Moerman

Kevin-Mattheus-Moerman Aug 28, 2017

Member

Fantastic, congratulations @SebastianoF 🍰 ! Thanks @rougier 👍

Fantastic, congratulations @SebastianoF 🍰 ! Thanks @rougier 👍

@dzhoshkun

This comment has been minimized.

Show comment
Hide comment
@dzhoshkun

dzhoshkun Aug 29, 2017

@Kevin-Mattheus-Moerman @rougier @arfon Many thanks for your efforts and esp. the useful feedback we've received as part of the review process. Cheers.

@Kevin-Mattheus-Moerman @rougier @arfon Many thanks for your efforts and esp. the useful feedback we've received as part of the review process. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment