search

openjournals / joss-reviews

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

[REVIEW]: PyNeuroTrace - Python code for neural activity time series #6877

Closed editorialbot closed 1 month ago

editorialbot commented 3 months ago

Submitting author: !--author-handle-->@peter-hogg<!--end-author-handle-- (Peter William Hogg) Repository: https://github.com/padster/pyNeuroTrace Branch with paper.md (empty if default branch): Version: 1.0.1 Editor: !--editor-->@adamltyson<!--end-editor-- Reviewers: @stephenlenzi, @danielmk Archive: 10.5281/zenodo.13312905

Status

status

Status badge code:

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

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

@stephenlenzi & @danielmk, 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 @danielmk

📝 Checklist for @stephenlenzi

editorialbot commented 3 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 3 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.7554/eLife.38173 is OK
- 10.1371/journal.pcbi.1005423 is OK
- 10.1038/s41467-024-47515-x is OK
- 10.1038/s43586-022-00147-1 is OK
- 10.1371/journal.pone.0157595 is OK
- 10.1002/jbio.201100133 is OK
- 10.1038/nprot.2010.169 is OK
- 10.1038/s41592-019-0597-2 is OK
- 10.1109/embc.2018.8512983 is OK
- 10.3389/fncir.2020.00033 is OK
- 10.1016/s0896-6273(01)00235-5 is OK

MISSING DOIs

- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...

INVALID DOIs

- None
editorialbot commented 3 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.06 s (1267.7 files/s, 223598.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                              1              0              0           2671
HTML                            13            310             39           2194
Python                          17            466            955           1394
CSS                              6            339             51           1324
Markdown                         5            365              0           1014
JavaScript                      12            138            228            911
YAML                             4             20              7            151
TeX                              1             11              0            147
reStructuredText                11             35             62             40
TOML                             1              7              0             30
DOS Batch                        1              8              1             26
Jupyter Notebook                 1              0            238             24
make                             1              4              7              9
INI                              1              0              0              3
-------------------------------------------------------------------------------
SUM:                            75           1703           1588           9938
-------------------------------------------------------------------------------

Commit count by author:

    69  Pat Coleman
    44  peter-hogg
    27  padster
    22  Peter
     4  Peter Hogg
editorialbot commented 3 months ago

Paper file info:

📄 Wordcount for paper.md is 2309

✅ The paper includes a Statement of need section

editorialbot commented 3 months ago

License info:

✅ License found: Mozilla Public License 2.0 (Valid open source OSI approved license)

adamltyson commented 3 months ago

@stephenlenzi, @danielmk, 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#6877 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.

editorialbot commented 3 months ago

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

danielmk commented 3 months ago

Review checklist for @danielmk

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

adamltyson commented 3 months ago

Hi @danielmk, @stephenlenzi :wave:. Just checking in to see how the review is going. Let me know if I can be of any help.

danielmk commented 2 months ago

I started the review but I am currently on conference travel, so I won't be able to continue working on it until next week.

On Mon, Jun 24, 2024, 10:26 Adam Tyson @.***> wrote:

Hi @danielmk https://github.com/danielmk, @stephenlenzi https://github.com/stephenlenzi 👋. Just checking in to see how the review is going. Let me know if I can be of any help.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/6877#issuecomment-2185908214, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD3QX433A26R25GKPWPXDXLZI7J2VAVCNFSM6AAAAABJE74GF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVHEYDQMRRGQ . You are receiving this because you were mentioned.Message ID: @.***>

stephenlenzi commented 2 months ago

Thanks @adamltyson all good here, I should make some progress on this over the weekend and next week.

dmikushin commented 2 months ago

Hi @Peter-Hogg , could you please fix broken links to images in the README.md file?

Peter-Hogg commented 2 months ago

Hi @Peter-Hogg , could you please fix broken links to images in the README.md file?

I can do that

Peter-Hogg commented 2 months ago

@dmikushin fixed

stephenlenzi commented 2 months ago

Review checklist for @stephenlenzi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

stephenlenzi commented 2 months ago

Hi @Peter-Hogg in the example notebook _, _ = plotIntensity(rawTimeSeries, hz, title='Cal590 Raw Traces') fails because plotIntensity is not imported. Could you add the import?

Also, could you please add jupyter as a dependency? Alternatively since it is not strictly needed for the software but only for the examples you could indicate in the README that the user needs to pip install jupyter to use the notebooks. I realise its obvious to most, but I suspect not everyone will realise this.

adamltyson commented 2 months ago

Hi @danielmk, @stephenlenzi - how is the review going? Let me know if I can help with anything.

danielmk commented 2 months ago

Review going well. Mainly waiting for code and data to confirm the GPU performance. See https://github.com/padster/pyNeuroTrace/issues/7

stephenlenzi commented 2 months ago

Hi @adamltyson same here. All going well just awaiting the outcome of @danielmk's issue padster/pyNeuroTrace#7 so I can check off the performance and gpu-related items

Peter-Hogg commented 2 months ago

Sorry for the delay I'm currently traveling. It's been a busier trip than I expected. I'm back home this weekend and will be able to push the extra notebooks and code to the repo.

adamltyson commented 1 month ago

Hi all, just checking in again to see how this is going. It looks like only one issue remains on the GitHub repo?

stephenlenzi commented 1 month ago

Thanks @adamltyson I expect to finish this off today.

danielmk commented 1 month ago

Hello @adamltyson I just ran the GPU code so I can confirm the performance claim. That was my last major item. I opened one last issue regarding the source of the data and possible ethics approval. That should be easy to clarify and when that issue is closed I am through my checklist.

adamltyson commented 1 month ago

Thanks both @stephenlenzi, @danielmk, much appreciated!

Peter-Hogg commented 1 month ago

I want to thank the reviewers for their insights and feedback. I've added a statement on ethics approval to the manuscript. If the remaining issues on the repository have been addressed to a satisfactory level I'll close them.

adamltyson commented 1 month ago

Thanks @Peter-Hogg. @danielmk your checklist is complete, so you can you confirm you're happy with the changes to the manuscript, and you're happy to recommend acceptance?

@stephenlenzi is there anything outstanding on your side?

danielmk commented 1 month ago

@adamltyson Yes I recommend acceptance. I just want to note that I made the authors add a statement on animal ethics, since the in-vivo data in the paper is not published elsewhere. From my perspective the ethics statement is appropriate, but maybe JOSS has some specific guidelines that I am not aware of. Since you have more experience with JOSS, it would be great if you could have a quick look in the paper.pdf and confirm that the ethics statement is fine. Otherwise that's it from me and I am satisfied with the paper. lLt me know if I can help with anything else.

stephenlenzi commented 1 month ago

Thanks @adamltyson, there is nothing left outstanding on my side. I also recommend acceptance.

adamltyson commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

adamltyson commented 1 month ago

Hi @danielmk, @Peter-Hogg. If this data is original, then there are some specific JOSS guidelines. Thanks for adding the relevant approvals @Peter-Hogg, but would you also be able to report either the ARRIVE or PREPARE guidelines too?

Then it looks like we're ready to go. FYI I'm away for the rest of this week, so all being well I will start the acceptance process next week.

Thanks all!

Peter-Hogg commented 1 month ago

@adamltyson I hope you enjoy your trip. I'll work on the ARRIVE documentation. Should I add a methods section to the manuscript detailing sample preparation as well? That's currently not included in the manuscript as I thought it was outside of the scope of paper when I submitted it. However, these reports from ARRIVE and PREPARE would like them included to make the work more reproducible.

adamltyson commented 1 month ago

Yes, please add whatever is needed to adhere to the ARRIVE or PREPARE guidelines. It's of course not related to the software itself, but if this is original data, then it's JOSS policy that these details are added.

Peter-Hogg commented 1 month ago

@adamltyson I've add the changes to the manuscript and used the "The ARRIVE guidelines 2.0: author checklist". The manuscript adheres to the ARRIVE guidelines.

Peter-Hogg commented 1 month ago

@editorialbot generate pdf ~edit The proof generated from this is missing the changes. Perhaps I called this too soon

editorialbot commented 1 month ago

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

adamltyson commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

adamltyson commented 1 month ago

@editorialbot check references

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

OK DOIs

- 10.7554/eLife.38173 is OK
- 10.1371/journal.pcbi.1005423 is OK
- 10.1038/s41467-024-47515-x is OK
- 10.1038/s43586-022-00147-1 is OK
- 10.1371/journal.pone.0157595 is OK
- 10.1002/jbio.201100133 is OK
- 10.1038/nprot.2010.169 is OK
- 10.1038/s41592-019-0597-2 is OK
- 10.1109/embc.2018.8512983 is OK
- 10.3389/fncir.2020.00033 is OK
- 10.1016/s0896-6273(01)00235-5 is OK
- 10.1101/061507 is OK
- 10.1101/pdb.prot106831 is OK
- 10.1038/s41598-017-02301-2 is OK

MISSING DOIs

- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...
- No DOI given, and none found for title: SLAP2 – Two Photon Microscope Kit

INVALID DOIs

- None
adamltyson commented 1 month ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

adamltyson commented 1 month ago

Thanks @Peter-Hogg. I will give the paper itself a once-over. In the meantime, would you be able to go through the list of "Additional Author Tasks" above?

Could you also double check the missing DOIs too, i.e. check if a DOI can be added? I think they're probably OK because I seem to remember that NeurIPS doesn't give DOIs, and one is a commercial product.

adamltyson commented 1 month ago

The paper looks good @Peter-Hogg. I've raised a PR with some tiny changes then I think we're ready to go.

I also raised an issue about making the docs more obvious. It's not required for acceptance, but I think it would be very helpful for users!

Peter-Hogg commented 1 month ago

@editorialbot set 10.5281/zenodo.13208759 as archive

editorialbot commented 1 month ago

I'm sorry @Peter-Hogg, I'm afraid I can't do that. That's something only editors are allowed to do.

Peter-Hogg commented 1 month ago

I've archived the version 1.0.1 of the software on Zendo. (10.5281/zenodo.13208759)

DOI

I also can't find a DOI for Cupy to add as a reference. There isn't one in their manuscript.

Peter-Hogg commented 1 month ago

I think we're all good now

adamltyson commented 1 month ago

@editorialbot generate pdf

adamltyson commented 1 month ago

@editorialbot check references

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

OK DOIs

- 10.7554/eLife.38173 is OK
- 10.1371/journal.pcbi.1005423 is OK
- 10.1038/s41467-024-47515-x is OK
- 10.1038/s43586-022-00147-1 is OK
- 10.1371/journal.pone.0157595 is OK
- 10.1002/jbio.201100133 is OK
- 10.1038/nprot.2010.169 is OK
- 10.1038/s41592-019-0597-2 is OK
- 10.1109/embc.2018.8512983 is OK
- 10.3389/fncir.2020.00033 is OK
- 10.1016/s0896-6273(01)00235-5 is OK
- 10.1101/061507 is OK
- 10.1101/pdb.prot106831 is OK
- 10.1038/s41598-017-02301-2 is OK

MISSING DOIs

- No DOI given, and none found for title: CuPy: A NumPy-Compatible Library for NVIDIA GPU Ca...
- No DOI given, and none found for title: SLAP2 – Two Photon Microscope Kit

INVALID DOIs

- None