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

[PRE REVIEW]: GIRFReco.jl: An open-source pipeline for spiral MRI reconstruction in Julia #5515

Closed
editorialbot opened this issue Jun 2, 2023 · 51 comments
Assignees
Labels
Julia pre-review Shell Track: 2 (BCM) Biomedical Engineering, Biosciences, Chemistry, and Materials XSLT

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Jun 2, 2023

Submitting author: @alexjaffray (Alexander Jaffray)
Repository: https://github.com/BRAIN-TO/GIRFReco.jl
Branch with paper.md (empty if default branch):
Version: v0.1.0
Editor: @Kevin-Mattheus-Moerman
Reviewers: @cncastillo, @mathieuboudreau, @felixhorger
Managing EiC: Kevin M. Moerman

Status

status

Status badge code:

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

Author instructions

Thanks for submitting your paper to JOSS @alexjaffray. Currently, there isn't a JOSS editor assigned to your paper.

@alexjaffray if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). You can search the list of people that have already agreed to review and may be suitable for this submission.

Editor instructions

The JOSS submission bot @editorialbot is here to help you find and assign reviewers and start the main review. To find out what @editorialbot can do for you type:

@editorialbot commands
@editorialbot editorialbot added pre-review Track: 2 (BCM) Biomedical Engineering, Biosciences, Chemistry, and Materials labels Jun 2, 2023
@editorialbot
Copy link
Collaborator Author

Hello human, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

Software report:

github.com/AlDanial/cloc v 1.88  T=0.05 s (713.7 files/s, 116317.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           24           1027            915           1940
XSLT                             1             58             30            840
TeX                              1             34              0            473
Markdown                         4             54              0            150
Bourne Shell                     1              6              1             52
TOML                             1              2              0             52
YAML                             3              9             12             49
-------------------------------------------------------------------------------
SUM:                            35           1190            958           3556
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

@editorialbot
Copy link
Collaborator Author

Wordcount for paper.md is 2231

@editorialbot
Copy link
Collaborator Author

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1002/mrm.23217 is OK
- 10.1002/mrm.24263 is OK
- 10.1002/mrm.28554 is OK
- 10.1002/jmri.20320 is OK
- 10.1109/TMI.2002.808360 is OK
- 10.1002/mrm.22767 is OK
- 10.1002/mrm.25827 is OK
- 10.1002/mrm.1241 is OK
- 10.1137/141000671 is OK
- 10.1002/mrm.26089 is OK
- 10.1002/mrm.25841 is OK
- 10.1002/mrm.24389 is OK
- 10.5281/ZENODO.592960 is OK
- 10.1002/mrm.29384 is OK
- 10.1002/mrm.28792 is OK
- 10.1002/mrm.24751 is OK
- 10.1109/TMI.2008.2006526 is OK
- 10.1109/TMI.2008.923956 is OK
- 10.1016/j.mri.2020.06.005 is OK
- 10.1002/mrm.27583 is OK
- 10.1109/TCI.2020.3031082 is OK
- 10.1002/mrm.1910390218 is OK
- 10.5281/zenodo.6510021 is OK
- 10.1002/mrm.27176 is OK
- 10.1016/j.neuroimage.2021.118674 is OK
- 10.1016/j.neuroimage.2017.07.062 is OK
- 10.1016/j.neuroimage.2021.118738 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot query scope

@editorialbot
Copy link
Collaborator Author

Submission flagged for editorial review.

@editorialbot editorialbot added the query-scope Submissions of uncertain scope for JOSS label Jun 9, 2023
@Kevin-Mattheus-Moerman
Copy link
Member

@alexjaffray thanks for this submission. I am the AEiC for this track and here to help process initial steps. Note that I've just flagged this submission for a scope review by the editorial board. This is normal for submissions that are relatively small in terms of size (lines of code) and/or those that appear rather new/immature and may be lacking in terms of "packaging". The scope review should take about 2 weeks to complete.

@jbytecode
Copy link

jbytecode commented Jun 26, 2023

@alexjaffray - I am a JOSS editor, and I am here to help you make your Julia package canonical before taking any further action. When I followed the README.md file in the software repository, I noticed a set of installation instructions that are not considered canonical for a Julia package. It doesn't even include a test folder and unit tests.

Could you please:

  • Add a test folder and implement tests. You can refer to the documentation for testing capabilities if you are unsure: https://docs.julialang.org/en/v1/stdlib/Test/#Unit-Testing.
  • Register the Julia package on JuliaHub so that users can install the package using the Pkg package in Julia: docs.
  • Add GitHub Actions for online continuous integration: Here is an example.
  • Update the README file as follows:

To install the package, use the following commands:

julia> using Pkg
julia> Pkg.add(pkgname)

or

julia> ]
(@v1.9) pkg> add pkgname

etc.

Please don't hesitate to ask me anything if you encounter any difficulties while applying any of the steps mentioned above.

@Kevin-Mattheus-Moerman
Copy link
Member

@alexjaffray can you get back to us on the above ☝️

@alexjaffray
Copy link

alexjaffray commented Jun 30, 2023 via email

@Kevin-Mattheus-Moerman
Copy link
Member

@alexjaffray great thanks for the update.

@Kevin-Mattheus-Moerman Kevin-Mattheus-Moerman added paused and removed query-scope Submissions of uncertain scope for JOSS labels Jul 7, 2023
@Kevin-Mattheus-Moerman
Copy link
Member

@alexjaffray I have just removed the query-scope label as based on the functionality this package offers this may be in scope. I did just now assign the paused label, just to flag that we will not resume until you've addressed the above. Please keep us up to date on progress, and if you can provide an estimate as to how long you think it will take, please do let us know.

@alexjaffray
Copy link

@Kevin-Mattheus-Moerman Hi, we have just package-ized the project.

The package and paper detail a pipeline, for which the integration test is the successful reconstruction of non-Cartesian MR imaging data. We provide details about how to carry out the integration test in the README and the Literate example, but please let us know if you need anything else regarding testing.

@jbytecode
Copy link

jbytecode commented Jul 29, 2023

I can now install the package using Julia package manager by typing

] add GIRFReco

however, when I am trying to perform the unit tests by typing

] test GIRFReco

I get an error, because the package doesn't have tests. Reviewers will already remind about the absence of unit tests during the review process and maybe we can pass this for now, right? @Kevin-Mattheus-Moerman

Additionally, you can update the README.md file of repository with new installation instructions by replacing manual installation by automatic package installation.

A note on the GitHub integration tests: Since your software hasn't tests, CI does not perform a full check of your software, and it is still absent.

By the way, we have a registered package, and I believe all of the issues will be addressed by the reviewers, and the author will eventually correct them. We can also remind them to focus on software testing.

@Kevin-Mattheus-Moerman
Copy link
Member

@alexjaffray can you get back to @jbytecode on the above ☝️ Looks like we are possible ready to proceed but if would be good if you commented on this.

@alexjaffray
Copy link

@jbytecode Thanks for the feedback, I will incorporate the suggestions and work on the testing of the package, I think the reviewers' input on a testing strategy will be very helpful.

@alexjaffray
Copy link

@jbytecode I've added the canonical installation instructions for the package, and we want to hear reviewer feedback on appropriate tests for this package. Feel free to proceed.

@jbytecode
Copy link

Looks great now. However, the package still lacks unit tests. I believe this issue will be emphasized in the review.

@Kevin-Mattheus-Moerman - I think this submission seems quite ready for review.

@Kevin-Mattheus-Moerman Kevin-Mattheus-Moerman added waitlisted Submissions in the JOSS backlog due to reduced service mode. and removed paused waitlisted Submissions in the JOSS backlog due to reduced service mode. labels Aug 8, 2023
@Laura2305
Copy link

I'm a PhD candidate and my supervisor is co-author of this paper so I have to decline.

@JakobAsslaender
Copy link

Unfortunately, I am also not available. Too many other commitments. Apologies!

@Kevin-Mattheus-Moerman
Copy link
Member

@Laura2305 thanks for declaring that conflict of interest.

@Kevin-Mattheus-Moerman
Copy link
Member

@grlee77, @mathieuboudreau, @spinicist, @timholy, @cncastillo, @garrettfullerton would you be interested in reviewing the following submission for the Journal of Open Source Software (JOSS). The submission is entitled: "GIRFReco.jl: An open-source pipeline for spiral MRI reconstruction in Julia"?

As you may know JOSS reviews focus on the the evaluation of the the software, and the review of a short paper.

Please let me know if you are interested by responding here. Thanks!

JOSS reviews take place on GitHub using a streamlined process, for more information see our review guidelines. You can also look at this completed review process to see what the process looks like: #1725.

@spinicist
Copy link

Apologies, I also do not have time. I tried asking two colleagues with experience in the area and sadly they are busy with other reviews at the moment.

@mathieuboudreau
Copy link

I've never coded in Julia and though I'm very experience in MRI (14+ years), I've not dealt tooo much with image recon aglorithms (and not spiral, besides basic theory). But, if that's ok, I'm happy to review

@cncastillo
Copy link

cncastillo commented Sep 14, 2023 via email

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot add @cncastillo as reviewer

@editorialbot
Copy link
Collaborator Author

@cncastillo added to the reviewers list!

@Kevin-Mattheus-Moerman
Copy link
Member

@alexjaffray as you can see I've been looking for reviewers. If you had any other suggestions of people (or communities on GitHub) I should check out, do let me know.

@alexjaffray
Copy link

@Kevin-Mattheus-Moerman I will consult the other authors and see if they have any more suggestions, and will update if they have other suggestions.

@mathieuboudreau
Copy link

I've never coded in Julia and though I'm very experience in MRI (14+ years), I've not dealt tooo much with image recon aglorithms (and not spiral, besides basic theory). But, if that's ok, I'm happy to review

Guess that's a no from you to me?

@mrikasper
Copy link

I've never coded in Julia and though I'm very experience in MRI (14+ years), I've not dealt tooo much with image recon aglorithms (and not spiral, besides basic theory). But, if that's ok, I'm happy to review

Guess that's a no from you to me?

@mathieuboudreau, thank you very much for offering to review, much appreciated. Maybe your modesty/disclaimer was dissuading @Kevin-Mattheus-Moerman ? Personally, I would be interested to see how a Julia-newbie would be able to use the tool, and you could dig deep into lack of documentation etc., but I am not sure what JOSS' expectations are exactly.

@mrikasper
Copy link

Other than that, @Kevin-Mattheus-Moerman, we could ask

  • the MRIReco.jl core developers: tknopp or migrosser
  • the ROMEO.jl MR image processing developers: korbinian90

@Kevin-Mattheus-Moerman
Copy link
Member

Kevin-Mattheus-Moerman commented Sep 18, 2023

@mathieuboudreau sorry I had not responded to you directly. Actually I'd be happy to have you review here too, it does sound like you may be able to provide useful feedback. In my mind I was considering you as an $(n+1)^{th}$ reviewer, where $n\geq2$, given that you said you never coded in Julia. So I'll assign/add you as a reviewer now, and we'll look for at least one more before we start. Thanks for volunteering!

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot add @mathieuboudreau as reviewer

@editorialbot
Copy link
Collaborator Author

@mathieuboudreau added to the reviewers list!

@Kevin-Mattheus-Moerman
Copy link
Member

@tknopp, @korbinian90, @grlee77, @timholy, @garrettfullerton, @felixhorger, @neuropoly would you be interested in reviewing the following submission for the Journal of Open Source Software (JOSS). The submission is entitled: "GIRFReco.jl: An open-source pipeline for spiral MRI reconstruction in Julia"?

As you may know JOSS reviews focus on the the evaluation of the the software, and the review of a short paper.

Please let me know if you are interested by responding here. Thanks!

JOSS reviews take place on GitHub using a streamlined process, for more information see our review guidelines. You can also look at this completed review process to see what the process looks like: #1725.

@korbinian90
Copy link

I would be interested

@korbinian90
Copy link

korbinian90 commented Sep 20, 2023

There is a conflict of interest, as Lars Kasper and I are both co-authors on the Neurodesk paper (50 authors in total). We didn't interact directly when contributing to Neurodesk however.

@felixhorger
Copy link

I am interested! I have worked with GIRFs a couple of years ago. Glad to see more and more julia packages pop up in the MRI community :)

@Kevin-Mattheus-Moerman
Copy link
Member

@korbinian90 thanks for comment on the possible COI, you are off the hook for this one then.

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot add @felixhorger as reviewer

@editorialbot
Copy link
Collaborator Author

@felixhorger added to the reviewers list!

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot start review

@editorialbot
Copy link
Collaborator Author

OK, I've started the review over in #5877.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Julia pre-review Shell Track: 2 (BCM) Biomedical Engineering, Biosciences, Chemistry, and Materials XSLT
Projects
None yet
Development

No branches or pull requests