openjournals / joss-reviews

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

[REVIEW]: Spectral Connectivity: a python package for computing multitaper spectral estimates and frequency-domain brain connectivity measures on the CPU and GPU #4840

Closed editorialbot closed 1 year ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@edeno<!--end-author-handle-- () Repository: https://github.com/Eden-Kramer-Lab/spectral_connectivity Branch with paper.md (empty if default branch): Version: v1.1.0 Editor: !--editor-->@emdupre<!--end-editor-- Reviewers: @AJQuinn, @sappelhoff, @EtienneCmb Archive: 10.5281/zenodo.7416614

Status

status

Status badge code:

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

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

@AJQuinn & @sappelhoff & @EtienneCmb, 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 @emdupre 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 @AJQuinn

πŸ“ Checklist for @sappelhoff

πŸ“ Checklist for @EtienneCmb

editorialbot commented 2 years 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 2 years ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.10 s (331.1 files/s, 111252.9 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
Jupyter Notebook                  2              0           2763           2512
Python                           13            778           1355           2330
TeX                               1             25              0            315
DOS Batch                         1             36              2            243
make                              2             32              6            199
Markdown                          3             52              0            167
YAML                              5              9             20            113
reStructuredText                  5             32             52             43
Bourne Again Shell                1              1              0              2
--------------------------------------------------------------------------------
SUM:                             33            965           4198           5924
--------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 684

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

OK DOIs

- 10.1016/j.jneumeth.2014.11.012 is OK
- 10.1109/ICDSP.2007.4288544 is OK
- 10.1007/PL00007990 is OK
- 10.1007/s005290050005 is OK
- 10.1073/pnas.1017041108 is OK
- 10.48550/arXiv.2201.11941 is OK
- 10.1101/2021.02.03.429582 is OK
- 10.1016/j.neuroimage.2008.02.020 is OK
- 10.2307/2287238 is OK
- 10.1016/0013-4694(83)90235-3 is OK
- 10.3389/fnins.2013.00267 is OK
- 10.1007/BF00198091 is OK
- 10.1016/S0165-0270(03)00052-9 is OK
- 10.1007/978-3-662-58485-9_11 is OK
- 10.1002/(sici)1097-0193(1999)8:4<194::aid-hbm4>3.0.co;2-c is OK
- 10.1523/JNEUROSCI.0854-21.2021 is OK
- 10.1016/j.clinph.2004.04.029 is OK
- 10.1103/PhysRevLett.100.234101 is OK
- 10.1002/hbm.20346 is OK
- 10.3389/fnsys.2021.645709 is OK
- 10.1016/j.neuroimage.2011.01.055 is OK
- 10.1016/j.neuroimage.2010.01.073 is OK
- 10.3389/fnins.2013.00267 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

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

emdupre commented 2 years ago

Hi @AJQuinn, @sappelhoff, and @EtienneCmb πŸ‘‹ Thanks again for agreeing to review this submission ! The review will take place in this issue, and you can generate your individual reviewer checklists by asking editorialbot directly with @\editorialbot generate my checklist.

In working through the checklist, you're likely to have specific feedback on Spectral Connectivity. Whenever possible, please open relevant issues on the linked software repository (and cross-link them with this issue) rather than discussing them here. This helps to make sure that feedback is translated into actionable items to improve the software !

If you aren't sure how to get started, please see the Reviewing for JOSS guide -- and, of course, feel free to ping me with any questions !

sappelhoff commented 2 years ago

Review checklist for @sappelhoff

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

AJQuinn commented 2 years ago

@editorialbot generate my checklist

emdupre commented 2 years ago

Hm, you might need to just create a new comment @AJQuinn -- I'm not sure that it will respond to edited comments ! Sorry for the confusion.

AJQuinn commented 2 years ago

Review checklist for @AJQuinn

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

sappelhoff commented 1 year ago

@emdupre I have completed my review. All issues that I raised have been addressed to my full satisfaction. I believe that spectral_connectivity is a valuable package that will serve the community well, as already evidenced in several publications. I recommend this paper for publication in JOSS.

For details, please see my checklist and all my linked issues and PRs in this review thread.

EtienneCmb commented 1 year ago

Review checklist for @EtienneCmb

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

emdupre commented 1 year ago

πŸ‘‹ Hi @AJQuinn, I wanted to make sure you weren't facing issues in getting started on your review ? Please let me know if there's anything I can do to help, and thank you again for agreeing to review Spectral Connectivity !

emdupre commented 1 year ago

πŸ‘‹ Hi @EtienneCmb, I just wanted to check in and see how this review is going for you. Please let me know if I can answer any questions that might help in finalizing your review ! And thanks again for agreeing to review Spectral connectivity for JOSS.

AJQuinn commented 1 year ago

Hi @emdupre @edeno - Apologies for the long delay, my review is in progress and should be completed tomorrow. I'm impressed overall. This is a useful package though I think the documentation needs a bit of work.

I've added some issues on the docs and will complete some comments on the code and paper tomorrow.

emdupre commented 1 year ago

Thank you, @AJQuinn ! Please let me know when you have completed your review, or if you consider your first pass complete and you are waiting on author revisions.

I'm also just noting that I have been corresponding with @EtienneCmb via email, and he confirmed that he should have his finalized review within the next week at the latest.

AJQuinn commented 1 year ago

Hi @emdupre my review is finished. I'm impressed with the package. It works well and the code is high quality. There are some similar packages with overlapping functionality but I think spectral_connectivity is a strong addition to the field with several unique features. I felt parts of the documentation were lacking a bit and have outlined some suggestions for this in the linked comments. Happy to proceed once these are actioned.

EtienneCmb commented 1 year ago

Hi @emdupre, I completed my review. This package has several originalities justifying its existence and its importance, especially for the neurophysiological community. Among those originalities, the possibility to choose collapsing axes is an important feature for performing, for example, single-trial and dynamic estimations of the connectivity. As raised by @AJQuinn, the documentation can be improved. I still have two issues opened on GPU computations and comparison with other software.

EtienneCmb commented 1 year ago

Dear @emdupre, the last issue regarding performance claims has been addressed. From my side, this package checks all the boxes. I recommend this manuscript for publication in JOSS.

edeno commented 1 year ago

Hi @AJQuinn, I believe I have addressed your issues with the documentation. Please see the updated documentation.

edeno commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

emdupre commented 1 year ago

Thank you @edeno for actioning these reviewer comments, and @EtienneCmb and @sappelhoff (who I see I have not yet thanked !! πŸ™‡ ) for confirming your completed reviews !

While we wait for additional comments from @AJQuinn , I'll perform a few editorial checks.

emdupre commented 1 year ago

@editorialbot check references

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

OK DOIs

- 10.1016/j.jneumeth.2014.11.012 is OK
- 10.1109/ICDSP.2007.4288544 is OK
- 10.1007/PL00007990 is OK
- 10.1007/s005290050005 is OK
- 10.1073/pnas.1017041108 is OK
- 10.48550/arXiv.2201.11941 is OK
- 10.1101/2021.02.03.429582 is OK
- 10.1016/j.neuroimage.2008.02.020 is OK
- 10.2307/2287238 is OK
- 10.1016/0013-4694(83)90235-3 is OK
- 10.3389/fnins.2013.00267 is OK
- 10.1007/BF00198091 is OK
- 10.1016/S0165-0270(03)00052-9 is OK
- 10.1007/978-3-662-58485-9_11 is OK
- 10.1002/(sici)1097-0193(1999)8:4<194::aid-hbm4>3.0.co;2-c is OK
- 10.1523/JNEUROSCI.0854-21.2021 is OK
- 10.1016/j.clinph.2004.04.029 is OK
- 10.1103/PhysRevLett.100.234101 is OK
- 10.1002/hbm.20346 is OK
- 10.3389/fnsys.2021.645709 is OK
- 10.1016/j.neuroimage.2011.01.055 is OK
- 10.1016/j.neuroimage.2010.01.073 is OK
- 10.3389/fnins.2013.00267 is OK
- 10.3389/978-2-88919-608-1 is OK
- 10.22541/au.159363438.81020330 is OK

MISSING DOIs

- None

INVALID DOIs

- None
AJQuinn commented 1 year ago

Hi all - @edeno et al have gone above and beyond in addressing my comments and I'm very happy to recommend spectral_connectivity for publication.

The documentation changes are excellent - I think this will be a massive benefit for future users. Thank you for your efforts.

emdupre commented 1 year ago

Thank you for confirming, @AJQuinn !

@edeno , I've finished reviewing the software and paper, and I'm also very happy with the submission. I do have a few editorial requests on the paper itself :

emdupre commented 1 year ago

After making these edits, could you then please :

I can then move forward with processing the submission πŸš€

edeno commented 1 year ago

Hi @emdupre:

Thank you so much for your work and to all the reviewers. The process has been very educational.

I have:

emdupre commented 1 year ago

@editorialbot generate pdf

emdupre commented 1 year ago

@editorialbot set v1.1.0 as version

editorialbot commented 1 year ago

Done! version is now v1.1.0

editorialbot commented 1 year ago

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

emdupre commented 1 year ago

@editorialbot set 10.5281/zenodo.7416614 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.7416614

emdupre commented 1 year ago

Thank you, @edeno !

I noticed that the metadata on the archive has several small discrepancies from the paper metadata. In particular, could you please update the title to :

Spectral Connectivity: a python package for computing multitaper spectral estimates and frequency-domain brain connectivity measures on the CPU and GPU

and double-check that the author information lists names and surnames correctly ? From my rendering, these appear inverted in the final citation (i.e., excerpting from the bib file):

author = {Eric, Denovellis and
          Maxym, Myroshnychenko and
          Mehrad, Sarmashghi and
          Emily, Stephen},

You can update these without creating a new archive by editing the existing archive's metadata as described here.

edeno commented 1 year ago

Ah yes. Silly mistake by me. Should be fixed now.

emdupre commented 1 year ago

Thank you ! I see that the author name changes have updated but not the title of the archive. Could you please confirm that ?

edeno commented 1 year ago

Sorry, I misunderstood what the title referred to. It should be fixed now?

emdupre commented 1 year ago

Thank you ; this now looks right on my end !

I'm now happy to recommend Spectral Connectivity to the EIC team for publication, and I just want to add my congratulations to you on such an impressive effort ! πŸŽ‰

emdupre commented 1 year ago

@editorialbot recommend-accept

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

OK DOIs

- 10.1016/j.jneumeth.2014.11.012 is OK
- 10.1109/ICDSP.2007.4288544 is OK
- 10.1007/PL00007990 is OK
- 10.1007/s005290050005 is OK
- 10.1073/pnas.1017041108 is OK
- 10.48550/ARXIV.2201.11941 is OK
- 10.1101/2021.02.03.429582 is OK
- 10.1016/j.neuroimage.2008.02.020 is OK
- 10.2307/2287238 is OK
- 10.1016/0013-4694(83)90235-3 is OK
- 10.3389/fnins.2013.00267 is OK
- 10.1007/BF00198091 is OK
- 10.1016/S0165-0270(03)00052-9 is OK
- 10.1007/978-3-662-58485-9_11 is OK
- 10.1002/(sici)1097-0193(1999)8:4<194::aid-hbm4>3.0.co;2-c is OK
- 10.1523/JNEUROSCI.0854-21.2021 is OK
- 10.1016/j.clinph.2004.04.029 is OK
- 10.1103/PhysRevLett.100.234101 is OK
- 10.1002/hbm.20346 is OK
- 10.3389/fnsys.2021.645709 is OK
- 10.1016/j.neuroimage.2011.01.055 is OK
- 10.1016/j.neuroimage.2010.01.073 is OK
- 10.3389/fnins.2013.00267 is OK
- 10.3389/978-2-88919-608-1 is OK
- 10.22541/au.159363438.81020330 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

:wave: @openjournals/bcm-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/3787, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

edeno commented 1 year ago

I found a couple of small typos that I fixed. We are happy how it looks otherwise.

Kevin-Mattheus-Moerman commented 1 year ago

@edeno I am the AEiC for this track and here to help process this work for acceptance in JOSS. I have reviewed the archive and the paper and all seems in order. I only have one minor editorial point:

Please let me know when you've worked on the above. Thanks.

edeno commented 1 year ago

Hi @Kevin-Mattheus-Moerman,

I added the affiliation country to the archive and the paper.md. Please let me know if there's another place it should be added.

Kevin-Mattheus-Moerman commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

Kevin-Mattheus-Moerman commented 1 year ago

@edeno all looks good now so we'll proceed with acceptance. Thanks