openjournals / joss-reviews

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

[REVIEW]: Thresholdmann: A Web tool for interactively creating adaptive thresholds to segment MRI data. #6336

Closed editorialbot closed 4 months ago

editorialbot commented 7 months ago

Submitting author: !--author-handle-->@katjaq<!--end-author-handle-- (Katja Heuer) Repository: https://github.com/neuroanatomy/thresholdmann Branch with paper.md (empty if default branch): joss-paper-branch Version: v.1.0.0 Editor: !--editor-->@adamltyson<!--end-editor-- Reviewers: @sneakers-the-rat, @62442katieb, @anibalsolon Archive: 10.5281/zenodo.11080336

Status

status

Status badge code:

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

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

@sneakers-the-rat & @62442katieb & @anibalsolon, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @adamltyson 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

Checklists

📝 Checklist for @sneakers-the-rat

📝 Checklist for @anibalsolon

📝 Checklist for @62442katieb

editorialbot commented 7 months ago

Hello humans, 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 commented 7 months ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.05 s (529.1 files/s, 211173.9 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
JSON                             2              0              0           7232
JavaScript                       8            415            160           2476
HTML                             3             43             13            426
CSS                              1              8              3            285
SVG                              7              3              3            207
TeX                              1              9              0            109
Markdown                         5             42              0            102
YAML                             2              1              4             33
-------------------------------------------------------------------------------
SUM:                            29            521            183          10870
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 7 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cortex.2019.04.011 is OK
- 10.1038/s42003-023-05553-z is OK
- 10.3897/rio.2.e9113 is OK
- 10.21105/joss.02670 is OK
- 10.1016/j.neuroimage.2006.01.015 is OK
- 10.1016/j.neuron.2019.12.023 is OK
- 10.1016/j.neuron.2021.10.015 is OK
- 10.1016/j.neuroimage.2020.117519 is OK
- 10.5281/ZENODO.3402456 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 7 months ago

Wordcount for paper.md is 1109

editorialbot commented 7 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

adamltyson commented 7 months ago

👋🏼 @katjaq @sneakers-the-rat @62442katieb @anibalsolon, this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#6336 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@adamltyson) if you have any questions/concerns.

sneakers-the-rat commented 7 months ago

Review checklist for @sneakers-the-rat

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Issues and PRs

Review Comments

If there is one thing you can always say about @katjaq , @r03ert0 , and @ntraut 's code it's that it does exactly what it says it does with style, simplicity, and solidity. Thanks to the authors for this lovely tool. I am only familiar with working with fMRI data being a huge hassle of gigantic packages and cumbersome GUIs, and so this tool that you can hold in the palm of your hand and run anywhere is an absolute treat. I am a huge fan of the author's small web, single purpose tool philosophy, and this is another great entry in that saga.

I have said this a number of times in the issues, but I think this kind of web development is especially lovely to see in an era of increasingly bloated javascript frameworks that are difficult to maintain and reverse engineer. for this tool to literally be deployed as is from a static web server - not even a build stage! - shows how much more legroom the web browser as programming environment has. The code is capable, concise, and correct. very well done. Docs are good, tests are good, all claims made in the paper easily validated, fundamentals are sound.

The area of growth i have suggested for the authors is to use some of those tools to extend the functional modularity of their work into a bit more fluid programmatic modularity. They make excellent use of their prior work with mriviewer and muijs, but the use of global page state does put a little bit of a cap on how the excellent work they have done here can be reused. One of the virtues of their very modular approach is that it naturally lends itself to compositionality - it's already possible to mix and match their tools as part of a researcher's workflow, the next level would be to be able to mix and match them in downstream tools. I have written a skeleton draft of a transitional path there in an above linked PR for the authors to take or leave. Even without tools like webpack, ES6 modules are well supported by browsers, so the authors can keep the dream of the low-fi but high-function read/write web alive and make a lovely little ecosystem that builds and builds and builds on itself!

I don't have much more to say since the work speaks for itself. Thanks again to the authors for their work, i will continue to jump at the chance to use and look at any work y'all do - always a treat. They have addressed all my suggestions and i wholeheartedly endorse this for JOSS!

anibalsolon commented 7 months ago

Review checklist for @anibalsolon

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Issues and PRs

62442katieb commented 7 months ago

Review checklist for @62442katieb

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

adamltyson commented 6 months ago

Hi @sneakers-the-rat, @anibalsolon, @62442katieb it looks like you've all started your reviews. Please let me know if I can be of any assistance!

adamltyson commented 6 months ago

:wave: @anibalsolon, @62442katieb,

Just a quick remidner that we aim to have reviews completed within 2-4 weeks. Do you have an idea when you'll have time to look into this?

@sneakers-the-rat it looks like you've raised issues on the software repo (thanks!) and are awaiting a response. Let me know if I can help with anything.

Thanks! Adam

adamltyson commented 6 months ago

@anibalsolon, @62442katieb, @sneakers-the-rat sorry to ping you all again, but I wanted to check in and see how things are going?

@sneakers-the-rat it looks like you're mostly just waiting for @katjaq to get back to you re the issues raised?

katjaq commented 6 months ago

Hi @adamltyson . Thank you so much for the ping. <3 At first, we were waiting a bit for the reviews of @anibalsolon @62442katieb – but then we just went ahead and started tackling the issues raised in the awesome review by @sneakers-the-rat 🌟 and implemented the suggested improvements. We will keep pushing our updates in the coming days :) Thanks again for all your great comments @sneakers-the-rat . <3

sneakers-the-rat commented 5 months ago

yes hello sorry for the delay i'll try and finish up this evening :)

Kevin-Mattheus-Moerman commented 5 months ago

@sneakers-the-rat thanks for your help here, were you able to proceed?

Kevin-Mattheus-Moerman commented 5 months ago

@62442katieb, @anibalsolon just checking in to see how you are getting on with this review. Could you provide an update? Thanks for your help!

sneakers-the-rat commented 5 months ago

Finished with my review! My checklist updated with my final comments, pasting here to they're also in the thread of conversation. Thanks again to the authors for their work and for addressing all my questions and comments.

If there is one thing you can always say about @katjaq , @r03ert0 , and @ntraut 's code it's that it does exactly what it says it does with style, simplicity, and solidity. Thanks to the authors for this lovely tool. I am only familiar with working with fMRI data being a huge hassle of gigantic packages and cumbersome GUIs, and so this tool that you can hold in the palm of your hand and run anywhere is an absolute treat. I am a huge fan of the author's small web, single purpose tool philosophy, and this is another great entry in that saga.

I have said this a number of times in the issues, but I think this kind of web development is especially lovely to see in an era of increasingly bloated javascript frameworks that are difficult to maintain and reverse engineer. for this tool to literally be deployed as is from a static web server - not even a build stage! - shows how much more legroom the web browser as programming environment has. The code is capable, concise, and correct. very well done. Docs are good, tests are good, all claims made in the paper easily validated, fundamentals are sound.

The area of growth i have suggested for the authors is to use some of those tools to extend the functional modularity of their work into a bit more fluid programmatic modularity. They make excellent use of their prior work with mriviewer and muijs, but the use of global page state does put a little bit of a cap on how the excellent work they have done here can be reused. One of the virtues of their very modular approach is that it naturally lends itself to compositionality - it's already possible to mix and match their tools as part of a researcher's workflow, the next level would be to be able to mix and match them in downstream tools. I have written a skeleton draft of a transitional path there in an above linked PR for the authors to take or leave. Even without tools like webpack, ES6 modules are well supported by browsers, so the authors can keep the dream of the low-fi but high-function read/write web alive and make a lovely little ecosystem that builds and builds and builds on itself!

I don't have much more to say since the work speaks for itself. Thanks again to the authors for their work, i will continue to jump at the chance to use and look at any work y'all do - always a treat. They have addressed all my suggestions and i wholeheartedly endorse this for JOSS!

adamltyson commented 5 months ago

Thanks @sneakers-the-rat, much appreciated!

@anibalsolon @62442katieb, could you let me know when you will be able to complete the review?

katjaq commented 5 months ago

Yeyyy! We completed answering all changes as suggested by @sneakers-the-rat ! That was an amazing review experience.

We thank @sneakers-the-rat for all the great comments, suggestions and code contributions which have made our tool more robust, well documented, user- and contributor friendly and apt for being integrated into neuroscientific workflows. We answered the issues raised, implemented tests to cover all functionalities, include a demo dataset for quick tool exploration, improved user- and code documentation, have a test setup that works across many browsers, restricted control points to the data boundaries, and tested performance on an unusually large dataset. We thank the reviewer for showing us how to move towards programmatic modularity in the future with web packaging and refactoring our code to separate source files from the rest and deploy using GitHub actions.

62442katieb commented 5 months ago

Thanks @sneakers-the-rat, much appreciated!

@anibalsolon @62442katieb, could you let me know when you will be able to complete the review?

Hello! Apologies for the delay. I should be able to complete my review by the end of the week.

adamltyson commented 5 months ago

Hi @anibalsolon, could you provide an update? Do you know when you will be able to complete the review? Thanks!

62442katieb commented 5 months ago

I've completed my review and have no substantive questions/comments/concerns. What a neat, useful, and user-friendly tool! Amazing work as always, @katjaq, @r03ert0, & @ntraut!

adamltyson commented 5 months ago

Thanks @62442katieb and @sneakers-the-rat. I think after two glowing reviews we're good to go, but I will hold off for a few more days in case @anibalsolon has any comments.

@anibalsolon please complete your review by the end of this week, otherwise we will proceed with the reviews we have.

anibalsolon commented 4 months ago

Thank you @adamltyson and sorry for the delay. Thank you @katjaq, @r03ert0 & @ntraut for such great work! I've provided my feedback via Github issues. @sneakers-the-rat issues covered most of my questions.

adamltyson commented 4 months ago

Thanks @anibalsolon! As you've ticked off every part of the reviewer checklist, can I assume that you recommend acceptance, and the issues you've raised, while important, are not blockers?

anibalsolon commented 4 months ago

Yes @adamltyson -- they are non-blockers, I vouch for the acceptance.

adamltyson commented 4 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

adamltyson commented 4 months ago

@editorialbot generate pdf

adamltyson commented 4 months ago

@editorialbot check references

editorialbot commented 4 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cortex.2019.04.011 is OK
- 10.1038/s42003-023-05553-z is OK
- 10.3897/rio.2.e9113 is OK
- 10.21105/joss.02670 is OK
- 10.1016/j.neuroimage.2006.01.015 is OK
- 10.1016/j.neuron.2019.12.023 is OK
- 10.1016/j.neuron.2021.10.015 is OK
- 10.1016/j.neuroimage.2020.117519 is OK
- 10.5281/ZENODO.3402456 is OK

MISSING DOIs

- None

INVALID DOIs

- None
adamltyson commented 4 months ago

Hi @katjaq, this looks ready to go :tada:

Could you create a new tagged release of the software, archive it (on Zenodo, figshare, or other), and post the version number and archive DOI here? I'll then get the ball rolling with the rest.

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

katjaq commented 4 months ago

Thank you for your review comments, @anibalsolon . We answered to all the issues you opened, and implemented suggested improvements.

We thank again all our reviewers for their great comments and this amazing review experience <3. We have answered all suggestions and implemented the necessary changes.

We thank the editor for giving us the green light and will go ahead and release this version and archive it for the final publication of our manuscript.

katjaq commented 4 months ago

Dear @adamltyson, we merged all our changes from the review process from master into the JOSS paper branch.

We also modified the paper.md file to reflect the updated implementation for testing. Could you please regenerate the pdf? Other than this change, the layout of the above provided proof looked all good to us.

We checked all the items in the list you gave us. Here is the information:

adamltyson commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

adamltyson commented 4 months ago

@editorialbot check references

editorialbot commented 4 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cortex.2019.04.011 is OK
- 10.1038/s42003-023-05553-z is OK
- 10.3897/rio.2.e9113 is OK
- 10.21105/joss.02670 is OK
- 10.1016/j.neuroimage.2006.01.015 is OK
- 10.1016/j.neuron.2019.12.023 is OK
- 10.1016/j.neuron.2021.10.015 is OK
- 10.1016/j.neuroimage.2020.117519 is OK
- 10.5281/ZENODO.3402456 is OK

MISSING DOIs

- None

INVALID DOIs

- None
adamltyson commented 4 months ago

@editorialbot set v.1.0.0 as version

editorialbot commented 4 months ago

Done! version is now v.1.0.0

adamltyson commented 4 months ago

@editorialbot set 10.5281/zenodo.11080336 as archive

editorialbot commented 4 months ago

Done! archive is now 10.5281/zenodo.11080336

adamltyson commented 4 months ago

@editorialbot generate pdf

editorialbot commented 4 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

adamltyson commented 4 months ago

@editorialbot recommend-accept

editorialbot commented 4 months ago
Attempting dry run of processing paper acceptance...
editorialbot commented 4 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cortex.2019.04.011 is OK
- 10.1038/s42003-023-05553-z is OK
- 10.3897/rio.2.e9113 is OK
- 10.21105/joss.02670 is OK
- 10.1016/j.neuroimage.2006.01.015 is OK
- 10.1016/j.neuron.2019.12.023 is OK
- 10.1016/j.neuron.2021.10.015 is OK
- 10.1016/j.neuroimage.2020.117519 is OK
- 10.5281/ZENODO.3402456 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 4 months ago

:warning: Error preparing paper acceptance. The generated XML metadata file is invalid.

No declaration for attribute displaystyle of element mfrac
No declaration for attribute displaystyle of element mfrac
adamltyson commented 4 months ago

Hi @katjaq, based on this and this comment, it seems as if setting a fraction in displaystyle using \dfrac is not supported by the JOSS compilation tool. Could you change it to \frac and see if that fixes the problem?

katjaq commented 4 months ago

Hello @adamltyson . Yeyyyyy, thank you so much for setting publication in motion 😍 I changed it to \dfrac and it seems to look like before from what I can see in the GitHub actions :). So, 🤞 . Sorry for the hiccup.