search

openjournals / joss-reviews

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

[REVIEW]: SLIX: A Python package for fully automated evaluation of Scattered Light Imaging measurements on brain tissue #2675

Closed whedon closed 3 years ago

whedon commented 4 years ago

Submitting author: @Thyre (Jan André Reuter) Repository: https://github.com/3d-pli/SLIX Version: v1.2.1 Editor: @oliviaguest Reviewers: @matteomancini, @rly, @alexrockhill Archive: 10.5281/zenodo.4121953

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

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

@matteomancini & @rly, 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 @oliviaguest 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 @matteomancini

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @rly

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @alexrockhill

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

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

OK DOIs

- 10.1103/PhysRevX.10.021002 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

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

oliviaguest commented 4 years ago

Hey 👋 @matteomancini, @rly: this is where the review will take place. Please make sure to read the instructions above.

For any and all things worthy of discussion or comment, use this issue right here — so drop comments or questions for me, the author, etc., here. For any very code-specific things please feel free to start an issue on the repo of the code itself (if appropriate!) and link back to it from here. For an example of how this process plays out feel free to skim previous reviews, such as: #2285 and #2348. ☺️

oliviaguest commented 4 years ago

Hey @matteomancini, @rly — when you get a chance can you give me an ETA for your reviews, please?

matteomancini commented 4 years ago

Apologies for the delayed reply, I should be able to provide a review by Friday the 16th of October.

rly commented 4 years ago

@oliviaguest I'll have this done by October 8.

oliviaguest commented 4 years ago

@whedon remind @rly in 1 week

whedon commented 4 years ago

Reminder set for @rly in 1 week

alexrockhill commented 4 years ago

I take it I'm no longer needed to review, if so best of luck with the review process

oliviaguest commented 4 years ago

@alexrockhill I would appreciate your input here — are you free/able to review this?

alexrockhill commented 4 years ago

Sure, I can have this done in a week

oliviaguest commented 4 years ago

@whedon add @alexrockhill as reviewer

whedon commented 4 years ago

OK, @alexrockhill is now a reviewer

whedon commented 4 years ago

:wave: @rly, please update us on how your review is going.

oliviaguest commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

matteomancini commented 4 years ago

I had some time to go through the SLIX repo and I think it is overall a very nice tool. It was very easy to have it up and running, and the command line interface is quite immediate. I had some visualization issue but it was because of what I was using to view the maps and the authors were very quick to reply. One suggestion that I have is to upload the package to PyPI: the installation is already quite simple, but it would be even easier to just type pip install slix.

I think the main thing missing (to fully check both the substantial scholarly effort and the functionality documentation) is an explicit explanation of how to use the output maps to inform tracking procedures in order to e.g. reconstruct streamlines. This was brought up by @alexrockhill here: https://github.com/3d-pli/SLIX/issues/7 – I think that a full tutorial on how to do some sort of “SLI-based tractography” is outside the scope of this work, but it would be appropriate to practically explain how to use one of the maps in a tracking scenario. Also generating automatically the vectorial data showed in figure 2i with Python would be a good way to bridge to tracking.

Another very minor issue I mentioned in https://github.com/3d-pli/SLIX/issues/10 is to add a practical example of how to use the other main CLI tool, SLIXLineplotParameterGenerator. Apart from that, I think the README is quite detailed and the methods are well commented.

Finally, the paper is well written, and I think that the current functionality summary and the statement of need are concise and appropriate. The only thing missing (but it may be due to my lack of experience with light imaging literature) is any mention of other similar tools with the related references. It may be the case that SLIX is the first of its kind for Scattered Light Imaging, but if it is the case a couple of sentences could be spent on similar tools for collateral techniques (e.g. polarized light imaging).

In any case, I want to stress that it is a very interesting tool!

Thyre commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

PDF failed to compile for issue #2675 with the following error:

/app/vendor/bundle/ruby/2.4.0/gems/octokit-4.8.0/lib/octokit/response/raise_error.rb:16:in on_complete': GET https://api.github.com/repos/JuliaCon/proceedings-review/issues/2675: 404 - Not Found // See: https://docs.github.com/rest/reference/issues#get-an-issue (Octokit::NotFound) from /app/vendor/bundle/ruby/2.4.0/gems/faraday-0.15.4/lib/faraday/response.rb:9:inblock in call' from /app/vendor/bundle/ruby/2.4.0/gems/faraday-0.15.4/lib/faraday/response.rb:61:in on_complete' from /app/vendor/bundle/ruby/2.4.0/gems/faraday-0.15.4/lib/faraday/response.rb:8:incall' from /app/vendor/bundle/ruby/2.4.0/gems/octokit-4.8.0/lib/octokit/middleware/follow_redirects.rb:73:in perform_with_redirection' from /app/vendor/bundle/ruby/2.4.0/gems/octokit-4.8.0/lib/octokit/middleware/follow_redirects.rb:61:incall' from /app/vendor/bundle/ruby/2.4.0/gems/faraday-0.15.4/lib/faraday/rack_builder.rb:143:in build_response' from /app/vendor/bundle/ruby/2.4.0/gems/faraday-0.15.4/lib/faraday/connection.rb:387:inrun_request' from /app/vendor/bundle/ruby/2.4.0/gems/faraday-0.15.4/lib/faraday/connection.rb:138:in get' from /app/vendor/bundle/ruby/2.4.0/gems/sawyer-0.8.2/lib/sawyer/agent.rb:94:incall' from /app/vendor/bundle/ruby/2.4.0/gems/octokit-4.8.0/lib/octokit/connection.rb:156:in request' from /app/vendor/bundle/ruby/2.4.0/gems/octokit-4.8.0/lib/octokit/connection.rb:19:inget' from /app/vendor/bundle/ruby/2.4.0/gems/octokit-4.8.0/lib/octokit/client/issues.rb:114:in issue' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-d14a699185fb/lib/whedon/review.rb:21:inissue_body' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-d14a699185fb/bin/whedon:44:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-d14a699185fb/bin/whedon:131:in<top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

Thyre commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

miriammenzel commented 4 years ago

Finally, the paper is well written, and I think that the current functionality summary and the statement of need are concise and appropriate. The only thing missing (but it may be due to my lack of experience with light imaging literature) is any mention of other similar tools with the related references. It may be the case that SLIX is the first of its kind for Scattered Light Imaging, but if it is the case a couple of sentences could be spent on similar tools for collateral techniques (e.g. polarized light imaging).

@matteomancini: Thank you very much for your positive comment and your constructive feedback. Scattered Light Imaging is indeed a very novel technique. It has been recently developed by our group and first introduced in Menzel et al. PRX 10:021002 (2020). It is the first technique of its kind, using light scattering in the optical range to obtain information about neural organization in the brain. Therefore, there exist no similar tools to SLIX that we could mention here. I have added two references about polarized light imaging at the beginning of the paper. Unfortunately, we already reached the maximum amount of words for the paper. Please let us know if we should still add more detail there (and what other information we could leave out instead). Concerning the other things you mentioned (issues #7,10,11), we have already replied to them or are in the process of implementing them.

alexrockhill commented 4 years ago

I largely agree with @matteomancini that SLIX serves a needed role in image processing for a new modality. My biggest critique that I have raised in issues thus far is that, although this software tool almost (directional plot PR pending AFAIK) replicates the authors' work on Bioarxiv, it doesn't seem as geared toward a new user who might have a slightly different setup and different needs as it could be. The authors have addressed these issues in the documentation, and I apologize for not responding sooner about their sufficiency, but I was planning to wait for @rly to finish the last review and then evaluate the process from start to finish again. I would finally add that the issue I raised about an example integration to a tractography software such as TRACULA and greater user control over parameters, are related to that, as a bit of a niche software (as opposed to a one-stop software), it could be made clearer where SLIX fits in a workflow that delivers a finished analysis. The main issue there being that there is more than one way to approach this image processing issue, and in my opinion, "Whether the software is sufficiently useful that it is likely to be cited by other researchers working in this domain" will depend on whether a researcher with good programming ability who collected this type of data would rather contribute to SLIX than start something new. I think SLIX is a package I personally would likely contribute to rather than starting something new if it were me, but more thorough documentation of why each analysis decision was taken in the software itself in a form that a user or developer could test and understand by working through themselves step-by-step would make this a non-decision. I don't think it's reasonable to expect SLIX to be a package with the stability and documentation of something like FSL or Freesurfer that have been around for decades in order to be published so I wouldn't set the bar at the issues brought up being completely resolved, but I think the groundwork and vision that the user base will expand and that SLIX could expand with it to meet an increasing diversity of needs would make SLIX do a better job of meeting this criteria. If it helps to have an example, I think this software tool https://pactools.github.io/ has a task that is similarly complex (there are multiple published ways to accomplish it), and it does a good job of walking a user/onboarding developer through step-by-step from data to results, has a flexible API and has the appearance, to me at least, that the methods could change or new methods could be integrated easily. Overall, I think the core algorithms seem well done and to do what is advertised and I think there is a SLIX-shaped hole in available software, but I wanted to ask the authors to consider a few comments about a more of a global perspective on SLIX's role in tractography software as well as smaller issues with the software.

rly commented 4 years ago

Sorry for the delay. I'll have by review done by end of day tomorrow and can join the discussion then.

miriammenzel commented 4 years ago

@alexrockhill: Thank you very much for your comments and sharing your thoughts. The purpose of SLIX is indeed to process the raw data from Scattered Light Imaging (SLI) and generate human-readable parameter maps. In some way, our software fulfills a similar purpose as software that transforms RF signals measured in an MR scanner to human-readable MRI images. As SLI is a completely new technique, SLIX is the first software of this kind and allows for the first time to evaluate the SLI measurements in a fully automated way and to study and interpret the scattering signals for the whole measured tissue sample. The SLI raw data is very complex and not easily interpretable. The purpose of SLIX is to process the raw data further, e.g. to extract the in-plane (crossing) nerve fiber orientations from the measured signals (peaks in the SLI profiles), and to provide interpretable parameter maps that can easily be processed/analyzed further.

We agree that the output of SLIX should be easily adaptable by the user to account for his/her diverse needs. Therefore, we provided “visualization.py” which contains different methods to visualize the output as described in our tutorial. The API is flexible and the output can be easily changed by the user, e.g. by replacing the function visualize_unit_vectors by something else. We added a Wiki, a full documentation of all available functions, and a brief statement concerning “Feature additions” in CONTRIBUTING.md. Concerning the input, it is already possible to perform the SLI measurement with arbitrary illumination angles and patterns, as long as the sample is illuminated under a fixed polar angle and different azimuthal angles in equidistant steps from 0° to 360°. These requirements are needed to perform a correct analysis of the SLI image stacks as described in the paper and should not be changed by the user.

We added a thorough documentation of why each analysis decision was taken in the software, both in the README.md and in the tutorial, and added options to change parameters (such as the 8% prominence, the 6% peak tip height, or the fraction of background pixels). In this way, the user can test the parameters and understand what they are used for. In the tutorial, we walk the user through step-by-step from the raw data (SLI image stack) to the results (parameter maps), and show different possibilities of visualization.

To demonstrate how SLIX fits in a workflow and could be used by tractography software, we added the section "Possible Integration in Tractography Algorithm" at the end of the tutorial, as suggested. There, we explain how the unit vector maps obtained by SLIX can be prepared to be used as input in streamline-based tractography algorithms (seed points propagating through the vector field). As we currently have no data set available that could be used as input (we would need SLI measurements of several consecutive brain sections to generate a sufficient 3D volume of unit vectors), we show an example where a 3D vector field (obtained by 3D Polarized Light Imaging) has been used as input for in-house developed tractography software.

We hope that we have now addressed all issues raised by the reviewers so that our software paper can be published. We have replied to all open issues and ask the reviewers to close them if our responses have been sufficient. @alexrockhill @matteomancini @rly: Please let us know if we should add anything else.

Thyre commented 4 years ago

I had some time to go through the SLIX repo and I think it is overall a very nice tool. It was very easy to have it up and running, and the command line interface is quite immediate. I had some visualization issue but it was because of what I was using to view the maps and the authors were very quick to reply. One suggestion that I have is to upload the package to PyPI: the installation is already quite simple, but it would be even easier to just type pip install slix.

I think the main thing missing (to fully check both the substantial scholarly effort and the functionality documentation) is an explicit explanation of how to use the output maps to inform tracking procedures in order to e.g. reconstruct streamlines. This was brought up by @alexrockhill here: 3d-pli/SLIX#7 – I think that a full tutorial on how to do some sort of “SLI-based tractography” is outside the scope of this work, but it would be appropriate to practically explain how to use one of the maps in a tracking scenario. Also generating automatically the vectorial data showed in figure 2i with Python would be a good way to bridge to tracking.

Another very minor issue I mentioned in 3d-pli/SLIX#10 is to add a practical example of how to use the other main CLI tool, SLIXLineplotParameterGenerator. Apart from that, I think the README is quite detailed and the methods are well commented.

Finally, the paper is well written, and I think that the current functionality summary and the statement of need are concise and appropriate. The only thing missing (but it may be due to my lack of experience with light imaging literature) is any mention of other similar tools with the related references. It may be the case that SLIX is the first of its kind for Scattered Light Imaging, but if it is the case a couple of sentences could be spent on similar tools for collateral techniques (e.g. polarized light imaging).

In any case, I want to stress that it is a very interesting tool!

Thank you for suggesting that SLIX could also be available under PyPI for a single-line installation process. I have not really thought about it before. While it took some time to implement all necessary steps to automate this process in the future, SLIX should be available here now. I tested the installation on two different machines, an empty Anaconda environment and a virtual environment. In every case the installation went smoothly and I had no problems using the library.

With the additions from this review I have increased the version number of SLIX from v1.1 to v1.2.1. I don't know if I am able to change the version number whedon posted at the first comment. Is it possible to change the version number to reflect the current implementation? Thanks a lot.

oliviaguest commented 4 years ago

@whedon set v1.2.1 as version

whedon commented 4 years ago

OK. v1.2.1 is the version.

oliviaguest commented 4 years ago

@Thyre absolutely! 😊 And done!

rly commented 4 years ago

I am reviewing the most recent version of the SLIX GitHub repo, which seems to include quite a few improvements since the last review: the addition of an example jupyter notebook, edits to the readme and paper, bug fixes, improved documentation, and release on PyPI.

Installation of SLIX was easy, both using PyPI and installing from source. Thank you for adding the option to install via pip. The software is an important tool for processing data from a new imaging technique - scattered light imaging. The paper is concise and well written, the examples run well, and the documentation is comprehensive. As mentioned by @matteomancini , the comparison to other tools and state of the field is lacking in the paper. I think it would be valuable to state clearly that to your knowledge, there is no other tool that does what SLIX does and that it is a necessary step for processing scattered light imaging data.

Is there source code for the command line executables in bin/? It should also be noted in the README that these executables cannot be run on Windows (at least I could not get them to run on Windows and used Linux instead).

Minor comments:

Thyre commented 4 years ago

Thanks so much for your comment. Both programs in /bin are actually Python files itself. You should be able to view the source code in the repository without any problems. I also agree that we should add a note that the program cannot be executed on Windows for now. This is unfortunately caused by pymp-pypi and will be fixed soon when the implementation of the GPU variant is finished. Until then, only users using macOS and Linux are able to run the program. A workaround on Windows is to use the WSL in the meantime.

I absolutely agree on all of your minor comments. We will fix them as soon as possible.

whedon commented 4 years ago

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

Thyre commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

miriammenzel commented 4 years ago

@rly: Thank you very much for your comments and your positive feedback. We have addressed all your concerns:

Please let us know if this meets your requirements or if there is anything else that we should add.

matteomancini commented 3 years ago

I think that both the tool and the paper now look much more complete. @Thyre I am glad to hear that the PyPI suggestion was useful! @miriammenzel I did not realise there was such a strict limit in terms of word count - taking that into account I think that the sentences added are sufficient to explain the difference between PLI and SLI, and the fact that SLIX is the first of its kind. All my comments have been addressed, and once the last minor things noticed by @rly are fixed, I think that the tool and the paper are both suitable for publication on JOSS.

alexrockhill commented 3 years ago

I agree that the tool looks to be in good shape and all the issues I have brought up have been addressed very well. I am happy to discuss when @rly is ready.

miriammenzel commented 3 years ago

Thank you @matteomancini @alexrockhill for your positive feedback. We are glad that we could address all your comments. I noticed that there are still some open issues in your review checklists? As mentioned in my previous post, we also tried to address all things mentioned by @rly - please let us know if we could satisfy your requirements.

Thanks again for your constructive feedback and the thorough review! It helped us a lot to improve our software and documentation.

alexrockhill commented 3 years ago

All checked just not up-to-date, just went through now.

matteomancini commented 3 years ago

Same for me, just updated it.

rly commented 3 years ago

@Thyre @miriammenzel Thanks for addressing my concerns! Everything looks good on my end. I think that the tool and the paper are both suitable for publication on JOSS.

oliviaguest commented 3 years ago

@whedon generate pdf

oliviaguest commented 3 years ago

@Thyre @miriammenzel can you deposit the code on an appropriate platform (e.g., zenodo), please? And then post the DOI here. 😊

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:

miriammenzel commented 3 years ago

@oliviaguest We have uploaded our software on zenodo (https://zenodo.org/record/4121953). DOI:10.5281/zenodo.4121953

oliviaguest commented 3 years ago

@whedon set 10.5281/zenodo.4121953 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4121953 is the archive.

oliviaguest commented 3 years ago

@Thyre @miriammenzel I think the repository on zenodo needs to have the same title as the paper, but otherwise, great! ☺️

oliviaguest commented 3 years ago

@whedon check references