search

openjournals / joss-reviews

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

[REVIEW]: MiTfAT: A Python-based scikit-learn-friendly fMRI Analysis Tool, Made in Tuebingen. #2827

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @vahid-sb (Vahid S. Bokharaie) Repository: https://github.com/vahid-sb/MiTfAT Version: v0.1.7 Editor: @arokem Reviewer: @JonathanReardon, @emdupre Archive: 10.5281/zenodo.4543316

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

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

@JonathanReardon & @emdupre, 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 @arokem 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 @JonathanReardon

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @emdupre

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper (vahid-sb/MiTfAT#3)

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @JonathanReardon, @emdupre 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 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- 10.1073/pnas.1908503116 may be a valid DOI for title: Early detection and monitoring of cerebral ischemia using calcium-responsive MRI probes

INVALID DOIs

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

arokem commented 3 years ago

:wave: @JonathanReardon and @emdupre -- just checking: have you had a chance to take a look at the paper, and start checking off those items? If you post issues on the software repo, please do add a pointer to this issue, so they are cross-linked.

emdupre commented 3 years ago

:wave: @JonathanReardon and @emdupre -- just checking: have you had a chance to take a look at the paper, and start checking off those items? If you post issues on the software repo, please do add a pointer to this issue, so they are cross-linked.

Thanks for checking-in, @arokem ! I've looked through the repo and code but I had not yet followed through the checklist. I'll start that today.

arokem commented 3 years ago

@JonathanReardon : have you had a chance to take a look?

arokem commented 3 years ago

@emdupre and @JonathanReardon : any progress here? Would be great to wrap up the first round of reviews before the holidays... (or even the whole thing? One can wish!).

emdupre commented 3 years ago

I'm so sorry about how long this has taken, @arokem and @vahid-sb ! I had some end-of-year deadline reshuffling (that I should have expected, but didn't). This is my top priority starting Wednesday morning, so I can commit to finishing my review at least a week before Christmas :)

JonathanReardon commented 3 years ago

Hi all. Sincere apologies for the delay back in December. I got sick unfortunately though I'm back on track now. I should get this done by Monday next week (11th Jan). Thank you for your patience and Happy New Year!

arokem commented 3 years ago

Hi @JonathanReardon -- thanks for the update! Glad that you are back on track. Having your feedback by next Monday would be great.

JonathanReardon commented 3 years ago

I have the software installed and all seems to be working as expected in terms of performance and functionality. In the README (first paragraph) it might help to be more explicit about what analysis methods you feel are missing from other software packages, and what software packages you are referring to. I would also re-frame the "belts and whistles" (*bells and whistles) sentence, it's quite vague and I think many would disagree that is the case. You could reword to suggest that other packages contain many options that can sometimes slow down the user (or something like that) -- just a suggestion.

From reading the README and manual, there appear to be quite a lot of grammatical errors and spelling mistakes, a final proofread would help. For example..

@vahid-sb

JonathanReardon commented 3 years ago

I couldn't see any guidelines for those wanting to contribute, report issues/problems etc. If that is not included, could you add it in? @vahid-sb

arokem commented 3 years ago

Hello @vahid-sb : have you had a chance to address the comments made by the reviewers? Thanks!

vahid-sb commented 3 years ago

Dear @JonathanReardon. Thanks for your suggestions. About the issues on the paper text, I have made a number of changes and corrections to the file, including removing or correcting the issues you highlighted. And I added CONTRIBUTING.md addressing how people can contribute.

vahid-sb commented 3 years ago

Dear @arokem, thanks again for accepting to be the editor for this paper. With the changes, I just made to the code and the documentation, I have addressed all the issues reviewers raised. Dear @emdupre and @JonathanReardon, my sincere thanks again for your help in making this library better and more useful.

JonathanReardon commented 3 years ago

Thank you for making those changes @vahid-sb. I've now completed my part of the review. Congratulations for the project, I'm happy to suggest this for publication. Cheers!

arokem commented 3 years ago

Thanks @JonathanReardon.

@emdupre : have your concerns been properly addressed in the revision? Can https://github.com/vahid-sb/MiTfAT/issues/1, https://github.com/vahid-sb/MiTfAT/issues/2 and https://github.com/vahid-sb/MiTfAT/issues/3 be closed?

emdupre commented 3 years ago

@emdupre : have your concerns been properly addressed in the revision? Can vahid-sb/MiTfAT#1, vahid-sb/MiTfAT#2 and vahid-sb/MiTfAT#3 be closed?

I've closed vahid-sb/MiTfAT#1 and vahid-sb/MiTfAT#3, but I still have two concerns that I've pointed to in vahid-sb/MiTfAT#2 ! If those are addressed then I'd be happy to recommend the package for publication.

Also, although i don't think this should hold up the software, I did want to point to two potential improvements re: package testing and contributing guidelines.

  1. Currently, the testing is done manually (i.e, it must be run by a developer). I confirmed on my own machine that it passes, but it would be great to have this running on a continuous integration framework like CircleCI !
  2. Also, the contributing guidelines are a bit sparse at present. If you're interested in expanding them, I would recommend pointing to or incorporating existing guidelines like those from scikit-learn or fMRIPprep.
vahid-sb commented 3 years ago

@emdupre for CONTRIBUITING.md, I also checked a few of the samples in libraries I know. LIbraries like scikit-learn have a huge contributing community and such detailed instructions are necessary. But given the fact that so far I have been the sole developer, did not want to put the possible contributors off with a long list of detailed instructions and kept it as simple as possible. If the community around MiTfAT grows, then I'd update the contribution guidelines. And for CI, it has been on my mind and is in my long-term plans, but I am sure you agree that it is not something that undermines the functionalities of the library at this stage.

emdupre commented 3 years ago

But given the fact that so far I have been the sole developer, did not want to put the possible contributors off with a long list of detailed instructions and kept it as simple as possible. If the community around MiTfAT grows, then I'd update the contribution guidelines. And for CI, it has been on my mind and is in my long-term plans, but I am sure you agree that it is not something that undermines the functionalities of the library at this stage.

Yes, as I mentioned : I don't want to hold up the package over this, but I do think these are both directions that could be expanded in the future. Clearly that future is coming quickly -- I see that you've already gotten a CI infrastructure set-up ! :slightly_smiling_face:

@arokem : I've now closed https://github.com/vahid-sb/MiTfAT/issues/2, and I'm happy to recommend this software for publication. Thank you to @vahid-sb for addressing my comments and for putting this project together !

arokem commented 3 years ago

@whedon generate pdf

arokem commented 3 years ago

@whedon check references

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:

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

OK DOIs

- None

MISSING DOIs

- 10.1073/pnas.1908503116 may be a valid DOI for title: Early detection and monitoring of cerebral ischemia using calcium-responsive MRI probes
- 10.21105/joss.01295 may be a valid DOI for title: NiBetaSeries: task related correlations in fMRI
- 10.31219/osf.io/ujxp6 may be a valid DOI for title: NiPreps: enabling the division of labor in neuroimaging beyond fMRIPrep

INVALID DOIs

- None
arokem commented 3 years ago

Hi @vahid-sb : Could you please add DOIs to your references? The recent message from whedon suggests DOIs for three of your articles, and I think that the DOI for the Frontiers article is 10.3389/fninf.2014.00014.

arokem commented 3 years ago

I have also added some comments/suggestions on the text of the paper in https://github.com/vahid-sb/MiTfAT/pull/4

vahid-sb commented 3 years ago

@emdupre, thanks for your kind words. And bout CI, just after I saw your comment I saw an ad for Circle CI on a completely unrelated website. Not a so subtle hint by the gods of the internet! I tried it and it gives an error while trying to install dependencies. I should have a look when I can to see how this can be fixed. And thanks again for all your constructive and useful comments.

vahid-sb commented 3 years ago

@arokem Thanks for bringing the errors to my attention. I also fixed a few other minor ones mainly to make the text read better and clarify a couple of points that I felt can be explained in a more clear manner. And also added citation info for scikit-learn and added doi for all. And then it occurred to me that I have not mentioned anything about visualization functions of the library, so I added a few lines about that too. Please let me know if there are any remaining outstanding issues.

arokem commented 3 years ago

Thank you! At this point could you please:

I can then move forward with accepting the submission.

vahid-sb commented 3 years ago

Dear @arokem, here is the doi of the requested release: 10.5281/zenodo.4543316 Please let me know if anything should be changed in the release or the meta-data. I also made a final minor change, correcting a typo in a keyword in the .md file. Please use the version I just uploaded to create the pdf file.

arokem commented 3 years ago

@openjournals/joss-eics : this is ready for your final review

arfon commented 3 years ago

@openjournals/joss-eics : this is ready for your final review

Thanks @arokem. We ask that editors run @whedon accept as the handoff to the EiC on rotation. This adds a special label (and makes the final proofs) for review too.

arfon commented 3 years ago

@whedon set 10.5281/zenodo.4543316 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4543316 is the archive.

arfon commented 3 years ago

@whedon 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.1073/pnas.1908503116 is OK
- 10.21105/joss.01295 is OK
- 10.31219/osf.io/ujxp6 is OK
- 10.5281/zenodo.1306215 is OK
- 10.3389/fninf.2014.00014 is OK

MISSING DOIs

- None

INVALID DOIs

- 10.5555/1953048.2078195 is INVALID
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/2088

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

@whedon accept deposit=true
arfon commented 3 years ago

@whedon accept deposit=true

whedon commented 3 years ago 
Doing it live! Attempting automated processing of paper acceptance...
whedon commented 3 years ago

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

whedon commented 3 years ago

🚨🚨🚨 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 :point_right: https://github.com/openjournals/joss-papers/pull/2096
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02827
  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...

arfon commented 3 years ago

@JonathanReardon, @emdupre - many thanks for your reviews here and to @arokem for editing this submission. JOSS relies upon the volunteer efforts of folks like yourselves and we simply wouldn't be able to do this without you! ✨

@vahid-sb - your paper is now accepted and published in JOSS :zap::rocket::boom:

whedon commented 3 years ago

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

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

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

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

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:

vahid-sb commented 3 years ago

I wish to sincerely thank you all for your contributions to this review process. Dear @arokem, thanks for accepting to be the editor and for your effort in organizing the review and for your own comments to improve the paper. Dear @emdupre and @JonathanReardon, thanks a lot for accepting to be a reviewer and for your very helpful comments which made this library and its documentation better and more accessible and useful for potential users. And dear @arfon, thanks for being the driving force behind JOSS. As somebody who has done quite a few reviews for various journals, I find this style of review the best and the most transparent and something all journals should aspire to achieve. Thanks again to you all.

arfon commented 3 years ago

Thanks for the kind words @vahid-sb!