search

openjournals / joss-reviews

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

[REVIEW]: NiTransforms: A Python tool to read, represent, manipulate, and apply n-dimensional spatial transforms #3459

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @mgxd (Mathias Goncalves) Repository: https://github.com/poldracklab/nitransforms Version: 21.0.0 Editor: @osorensen Reviewer: @robbisg, @PeerHerholz Archive: 10.5281/zenodo.5499694

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

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

@robbisg & @PeerHerholz, 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 @osorensen 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 @robbisg

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @PeerHerholz

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @robbisg, @PeerHerholz 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

Failed to discover a Statement of need section in paper

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5281/zenodo.591597 is OK
- 10.1016/j.neuroimage.2012.01.021 is OK
- 10.1016/j.neuroimage.2011.09.015 is OK
- 10.1016/j.media.2007.06.004 is OK

MISSING DOIs

- None

INVALID DOIs

- 10.1002/(SICI)1099-1492(199706/08)10:4/5  extless171::AID-NBM453  extgreater3.0.CO;2-L is INVALID
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.16 s (285.5 files/s, 85904.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          25            772            911           2479
SVG                              4              1              1           1368
YAML                             5             31              7            389
Jupyter Notebook                 3              0           7278            238
make                             1             30              6            148
Dockerfile                       1             18             15            125
Markdown                         2             17              0            100
reStructuredText                 4             16             28             77
TeX                              1              5              0             70
TOML                             1              1              0              9
-------------------------------------------------------------------------------
SUM:                            47            891           8246           5003
-------------------------------------------------------------------------------

Statistical information for the repository '3744697bb45cd68c5f464001' was
gathered on 2021/07/08.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Chris Markiewicz                 2             6              2            0.08
Mathias Goncalves                1             1              1            0.02
Oscar Esteban                   16           337             90            4.29
Stefano Moia                     1             1              0            0.01
mathiasg                        43          1873            181           20.66
oesteban                        66          4791           2591           74.24
smoia                            4            42             27            0.69

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
Chris Markiewicz              4           66.7         15.4                0.00
Oscar Esteban              3486         1034.4         16.2               12.65
mathiasg                    657           35.1         10.7               11.42
smoia                        15           35.7         19.5                6.67
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:

PeerHerholz commented 3 years ago

To the authors,

I sincerely hope you and your loved ones are still doing ok during these ongoing difficult times.

At first, I would like to thank all authors for their very interesting submission and hard, dedicated work they put into it. The topic and aim of this resource are both fascinating and important. Within the following I would like to provide additional information regarding my review decision, which will be divided into the following respective issues (also linked above) opened within the tool's repository:

In case some of my comments are misleading, difficult to understand or simply wrong based on an insufficient understanding on my side, I would like to apologize and kindly ask the authors to address such concerns. The same holds true for the nature of my comments: I aim to only provide helpful and constructive comments and criticism. If my wording appears too harsh or not helpful, please let me know and I will try my best to address this.

The presented tool, NiTransforms, allows researchers to work with an extensive amount and variety of transform formats. This includes both basic file handling (read/write/load/save), conversion between transform formats and application of transforms to images. Given the current state-of-the-art and increasing amount of workflows/pipelines that combine multiple software packages, the presented tool fills a prominent gap in the python neuroimaging ecosystem and will tremendously help to boost the reproducibility of neuroimaging results, while at the same time presenting the foundation for various other applications that could build upon it. Thus, it's clear "accept" from my side after the above outlined points were addressed/discussed.

Thanks again for this great contribution and please let me know if there are questions!

Cheers, Peer

robbisg commented 3 years ago

The proposed tool, nitransforms, is used to convert transformation files and manage them very easily.

I think that this tool is very useful and make it available will extremely help the python neuroimaging community. I agree with @PeerHerholz that this tool is very useful to ease building complex and automated pipelines and help the reproducibility of neuroimaging studies.

I opened an issue in the package repository (https://github.com/poldracklab/nitransforms/issues/123) with my suggestions. My suggestions are related to the documentation part, I encouraged the authors to add examples or tutorials, since from my perspective, this is the entry point of new users and a way to understand the usefulness of the software. Moreover a brief overview of other tools used for this purposes should be included in the paper, if no other tools are available, a clear statement that this is the only tool with these goals can help to add importance to it.

Once that these suggestions will be addressed, I will be happy to accept the publication to JOSS.

If you have any other questions, let me know. I would like to apologize if some of my comments are misleading.

Cheers, Roberto

osorensen commented 3 years ago

Thanks for the reviews, @robbisg and @PeerHerholz, and for opening issues in the source repository. @mgxd please take your time to resolve the issue, and feel free to reach out to me if there is anything I can help with.

whedon commented 3 years ago

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

whedon commented 3 years ago

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

osorensen commented 3 years ago

@mgxd, could you please update us on how it's going with the issues opened by the reviewers in the source repository?

mgxd commented 3 years ago

Hi all,

Thank you @PeerHerholz and @robbisg for the reviews! We have spent the past few weeks addressing concerns, and feel that we're at a good place - when you can, please give nitransforms another look.

Cheers, Mathias

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

PeerHerholz commented 3 years ago

Ahoi hoi folks,

thx so much for all the work on this @mgxd & @oesteban, I think the changes look great!

Two small things: as the contribution guidelines were added, I was wondering about the CoC and regarding the examples, I wanted to ask if a few sentences/explanations could be added to the Preparing Images and AFNI Deoblique notebooks, as well as functionality to download the examples either as .ipynb or .py?

Thanks again, cheers, Peer

osorensen commented 3 years ago

Thanks for all your work with this review @PeerHerholz! If you think that some of the issues on the review checklist have been fixed, please remember to check off the boxes.

PeerHerholz commented 3 years ago

Ah damn, completely forgot about that, thx @osorensen! I just checked the respective boxes.

oesteban commented 3 years ago

Sure, we'll get that done next week. Thanks much for an outstanding effort from both reviewers. Very much appreciated.

On Sat, Aug 21, 2021, 09:18 Peer Herholz @.***> wrote:

Ahoi hoi folks,

thx so much for all the work on this @mgxd https://github.com/mgxd & @oesteban https://github.com/oesteban, I think the changes look great!

Two small things: as the contribution guidelines were added, I was wondering about the CoC and regarding the examples, I wanted to ask if a few sentences/explanations could be added to the Preparing Images https://nitransforms.readthedocs.io/en/latest/notebooks/01_preparing_images.html and AFNI Deoblique https://nitransforms.readthedocs.io/en/latest/notebooks/02_afni_deoblique.html notebooks, as well as functionality to download the examples either as .ipynb or .py?

Thanks again, cheers, Peer

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3459#issuecomment-903073811, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAESDRSCYCNQY4BHSVBVM2DT55HMFANCNFSM5AAI2KBQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

robbisg commented 3 years ago

Hi everyone,

sorry for the late reply. Thank you for your changes. I agree with @PeerHerholz that examples need a few lines of description of what's going on in the script. I think that a brief overview (one sentence) of similar package or about the lack of tools like yours can be added to the paper.

For the rest, you did a very good job! ;)

Cheers, Roberto

osorensen commented 3 years ago

Dear @oesteban and @mgxd, could you please update us on how it is going responding to the last points raised by the reviewers?

oesteban commented 3 years ago

Hi @osorensen, yes, I'll be working on this today and tomorrow. It's been my oversight - thank you all for your patience (esp. Peer and Roberto)

oesteban commented 3 years ago

@PeerHerholz @robbisg @osorensen thanks much for your patience.

We've pushed the following changes:

@PeerHerholz I was wondering about the CoC

https://github.com/poldracklab/nitransforms/commit/be5341b5f37b83b25e398799908bc65ba4bf4b7b

@PeerHerholz a few sentences/explanations could be added to the Preparing Images and AFNI Deoblique notebooks, as well as functionality to download the examples either as .ipynb or .py? @robbisg examples need a few lines of description of what's going on in the script.

We have added the due descriptions and also included an automated prologue of the nbsphinx extension to include links to the source code of the examples:

@robbisg a brief overview (one sentence) of similar package or about the lack of tools like yours can be added to the paper.

poldracklab/nitransforms#132 (files)

Please let us know if there's anything else we might have missed in addressing all the comments. Thanks very much for the effort and time you've sunk into this, it's been really worthwhile on our side for improving the accessibility and usability of the project and the JOSS submission itself. Very much appreciated.

PeerHerholz commented 3 years ago

Ahoi hoi folks,

thx so much for all the work @oesteban & @mgxd, the changes look great!

IMHO the AFNI example could still use a bit more explanations, however, I think moving the comments out of the larger code blocks might already be enough. Sorry, I really don't mean to be nitpicking, but just trying to approach the materials from a potential user perspective. I would check my remaining box asap after that!

Cheers, Peer

oesteban commented 3 years ago

@PeerHerholz - what about https://github.com/poldracklab/nitransforms/pull/134 ? Do you think that would clarify things up?

robbisg commented 3 years ago

Hi,

I agree with @PeerHerholz ! it is one of the first things that I visit when a new tool is released.

Given your brand new fix, you can split also cell [79] in different titled cells, from my side.

The paragraph about other tools is ok! :)

Thank you for your work! Roberto

PeerHerholz commented 3 years ago

Ahoi hoi @oesteban,

damn that was fast! Yeah, looks great and comprehensible, thanks for adding the explanations. I'll check my last box and thus the submission is good to go from my end (@osorensen). Thanks again to the authors for all their great work and this important addition to the python-neuroimaging-world, I can't wait to work with NiTransforms!

Cheers, Peer

osorensen commented 3 years ago

Thanks to @PeerHerholz and @robbisg for the incredible job you have done with this review, and thanks to @oesteban for improving all the issues raised by the reviewers.

I will read through the paper once more now, and then get back to you with how to proceed.

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

osorensen 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/(SICI)1099-1492(199706/08)10:4/5<171::AID-NBM453>3.0.CO;2-L is OK
- 10.5281/zenodo.591597 is OK
- 10.1016/j.neuroimage.2012.01.021 is OK
- 10.1016/j.neuroimage.2011.09.015 is OK
- 10.1016/j.media.2007.06.004 is OK

MISSING DOIs

- None

INVALID DOIs

- None
osorensen commented 3 years ago

@oesteban, could you please fix the following issues in the paper?

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

osorensen commented 3 years ago

@oesteban before we proceed, could you please do the following?

oesteban commented 3 years ago

Sure thing, I just realized a self-citation was missed (fMRIPrep was the only piece of software mentioned without a due reference). I'll check on the Zenodo record - I believe we set it up, but I might be wrong. Anyways that was on the todo list down the line, so happy to get it right before finishing with the review stage.

osorensen commented 3 years ago

Good! I see there is a Zenodo archive for the paper (https://osf.io/8aq7b/), but please create one for the final version of the software too, so that we can get a doi link for the version that gets published.

oesteban commented 3 years ago

DOI of the archived version: 10.5281/zenodo.5499694

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

osorensen 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/(SICI)1099-1492(199706/08)10:4/5<171::AID-NBM453>3.0.CO;2-L is OK
- 10.5281/zenodo.591597 is OK
- 10.1016/j.neuroimage.2012.01.021 is OK
- 10.1016/j.neuroimage.2011.09.015 is OK
- 10.1016/j.media.2007.06.004 is OK
- 10.1038/s41592-018-0235-4 is OK

MISSING DOIs

- None

INVALID DOIs

- None
osorensen commented 3 years ago

@whedon set 10.5281/zenodo.5499694 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.5499694 is the archive.

osorensen commented 3 years ago

@whedon set 21.0.0 as version

whedon commented 3 years ago

OK. 21.0.0 is the version.

osorensen 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/(SICI)1099-1492(199706/08)10:4/5<171::AID-NBM453>3.0.CO;2-L is OK
- 10.5281/zenodo.591597 is OK
- 10.1016/j.neuroimage.2012.01.021 is OK
- 10.1016/j.neuroimage.2011.09.015 is OK
- 10.1016/j.media.2007.06.004 is OK
- 10.1038/s41592-018-0235-4 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/2582

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

@whedon accept deposit=true