openjournals / joss-reviews

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

[REVIEW]: PlatiPy: Processing Library and Analysis Toolkit for Medical Imaging in Python #5374

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@pchlap<!--end-author-handle-- (Phillip Chlap) Repository: https://github.com/pyplati/platipy Branch with paper.md (empty if default branch): joss-paper Version: v0.6.1 Editor: !--editor-->@samhforbes<!--end-editor-- Reviewers: @agisga, @tomelse Archive: 10.5281/zenodo.8032858

Status

status

Status badge code:

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

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

@agisga & @tomelse, 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 @samhforbes 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 @agisga

πŸ“ Checklist for @tomelse

editorialbot commented 1 year 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 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.30 s (1562.2 files/s, 180670.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          80           4231           3743          11283
CSS                              8           1278             50          10082
JavaScript                      11           2219           2022           9122
Jupyter Notebook                21              0           4758           1560
Markdown                        10            266              0            705
HTML                             5             60             15            609
reStructuredText                39            293            456            364
YAML                             7             26              5            279
SVG                            271              0              0            271
JSON                             3              1              0            143
TeX                              1             13              0            127
TOML                             1              8              0             76
Dockerfile                       7             37              3             50
DOS Batch                        1              8              1             26
Bourne Shell                     2              5              4             19
make                             1              4              7              9
INI                              1              0              0              3
-------------------------------------------------------------------------------
SUM:                           469           8449          11064          34728
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 1141

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

OK DOIs

- 10.3389/fninf.2013.00045 is OK
- 10.1007/s10278-017-0037-8 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.1371/journal.pone.0245364 is OK
- 10.1016/j.phro.2022.02.011 is OK
- 10.1016/j.meddos.2020.09.004 is OK
- 10.21105/joss.04555 is OK
- 10.1007/s10278-013-9622-7 is OK
- 10.1007/s13246-023-01231-w is OK

MISSING DOIs

- Errored finding suggestions for "Scikit-learn: Machine Learning in Python", please try later

INVALID DOIs

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

agisga commented 1 year ago

Review checklist for @agisga

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

tomelse commented 1 year ago

Review checklist for @tomelse

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

samhforbes commented 1 year ago

Hi @agisga & @tomelse - hope things are well. Do feel free to raise issues on the software repository if there's anything you feel needs addressing to complete your review, or if there's anything you'd like my input on please do write here.

agisga commented 1 year ago

@samhforbes sorry for the delay! I've been traveling / on vacation for the last three weeks. I will finish the review as soon as I return to work in one week.

tomelse commented 1 year ago

Really nice work, sorry for taking so long to do the review. I only have one recommendation I think, which would be to add some more detail (and references) to the paper about existing applications that do similar tasks. One example off the top of my head on the visualisation side, is MITK (https://www.mitk.org/wiki/The_Medical_Imaging_Interaction_Toolkit_(MITK)), which might be a good example to compare this to in your statement of need. If you could add a few examples like this and compare and contrast your toolkit to those, that would be really good to see.

samhforbes commented 1 year ago

Hi @pchlap, it looks like there's good feedback from @tomelse here, which should be straightforward to address. @agisga how are things looking?

agisga commented 1 year ago

I apologize for the delayed review. Overall, the platipy package appears to be in great shape. The documentation is sufficient, and the example Jupyter notebooks are well-designed and very helpful.

It is worth noting that platipy appears to be built on top of widely used lower-level imaging libraries, such as pydicom, SimpleITK and maybe others (at least in part). It may be beneficial to acknowledge these underlying packages and provide citations to the respective sources (e.g., I see citation of SimpleITK but not pydicom). However, discerning the exact extent to which platipy relies on pydicom, SimpleITK, or other libraries for its DICOM and image processing functionalities is somewhat challenging for me. It may be helpful to have a clearer understanding of these implementation details in the paper, but I leave it up to your judgement.

The JOSS review criteria (https://joss.readthedocs.io/en/latest/review_criteria.html) suggest including "A description of how this software compares to other commonly-used packages in this research area.". Thus, as also noted by the other reviewer, it would be beneficial to discuss alternative software solutions. For instance, in a paper on the Med-ImageTools Python package (https://f1000research.com/articles/12-118), the authors differentiate their solution from platipy and other alternatives. Although it is not a JOSS paper, it may be a useful reference related to this issue (note that I didn't read that paper in detail).

A few minor issues that I came across:

pchlap commented 1 year ago

Hi @samhforbes, @tomelse & @agisga,

thanks a lot for the really useful feedback. I will address your comments over the course of the next week or so and will let you know as soon as this is ready for another review.

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

pchlap commented 1 year ago

Hi @samhforbes, @tomelse & @agisga,

I have updated the paper based on your suggestions. Specifically I have added:

To respond to some of the other comments raised:

It is worth noting that platipy appears to be built on top of widely used lower-level imaging libraries, such as pydicom, SimpleITK and maybe others (at least in part). It may be beneficial to acknowledge these underlying packages and provide citations to the respective sources (e.g., I see citation of SimpleITK but not pydicom). However, discerning the exact extent to which platipy relies on pydicom, SimpleITK, or other libraries for its DICOM and image processing functionalities is somewhat challenging for me. It may be helpful to have a clearer understanding of these implementation details in the paper, but I leave it up to your judgement.

Yes this is a fair comment, platipy makes use of different libraries in different ways for the different components/modules. However I suppose it would be fair to say that we really do rely on SimpleITK across pretty much all components. Other libraries may only be used for certain parts. I do believe this comes across in the repository documentation but if you feel this piece of information would be useful in the paper I would be happy to add it.

Minor error when I run the last cell of dvh_analysis.ipynb: AttributeError: 'NoneType' object has no attribute 'rowspan' (Traceback points to pandas/plotting/_matplotlib/tools.py:396). But the visualization shows up anyway, so I didn't investigate further.

Ah this has just recently popped up due to an issue with pandas supporting the latest version of matplotlib. It seems that pandas is aware of the issue and will likely be fixed in an update. We could resolve this in our library by upper pinning matplotlib but I want to avoid that since platipy is a library others might include as a dependency and upper pins can be quite restrictive in those scenarios.

I couldn't execute generate_synthetic_head_neck_deformation.ipynb due to: SystemError: Animation functionality requires a matplotlib version lower than 3.4.0. I'm sure it would be easy to just downgrade matplotlib, but I didn't try. Perhaps there is a better way to deal with this at installation time? Personally, I don't think that this is a significant issue.

Yes I was aware of that one. Since this isn't a particularly popular piece of functionality in platipy, again we didn't want to set an upper pin for matplotlib 3.4.0. So we raise this error and the user can, as you suggest, just manually downgrade matplotlib to use this functionality. Ultimately we may look at deprecating and removing this function from platipy entirely in the future.

Thanks again for the useful comments and review of our work. I'd be happy to hear if you have any further questions or comments and would be glad to make further adjustments based on those.

Thanks Phil

samhforbes commented 1 year ago

Hi @agisga @tomelse please take a look at these responses when you are ready, and in particular whether these address the open issues and unitcked boxes in your reviewer checklist.

agisga commented 1 year ago

@samhforbes Yes, this addresses all of my open issues adequately.

@pchlap Minor grammar issue in the paper:

[MITK] provides a C++ library and a graphical to perform

This should say "graphical user interface" instead of "graphical"?

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

pchlap commented 1 year ago

@pchlap Minor grammar issue in the paper:

[MITK] provides a C++ library and a graphical to perform

This should say "graphical user interface" instead of "graphical"?

Thank you @agisga, I have corrected this grammar issue in the paper.

tomelse commented 1 year ago

Thank you for submitting a revised version of the paper. Looks great! I've just updated the checklist, let me know if there's anything more I need to do.

samhforbes commented 1 year ago

Thanks @agisga @tomelse!

Well done @pchlap - this is looking really good.

samhforbes commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

pchlap commented 1 year ago

Thanks @samhforbes, I have addressed the author tasks in the post-review checklist:

It seems like I can't check off the items in the check list myself, but can confirm that all 5 items under the author tasks are complete.

Please let me know if there is anything else needed from me. Thanks!

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

samhforbes commented 1 year ago

Hi @pchlap I can see an issue with the bib: Ghandourh, W., Dowling, J., Chlap, P., Oar, A., Jacob, S., Batumalai, V., & Holloway, L. (2021). Medical Dosimetry Assessing tumor centrality in lung stereotactic ablative body radiotherapy ( SABR ): the effects of variations in bronchial tree delineation and potential for automated methods. 46, 94–101. https://doi.org/10.1016/j.meddos.2020.09.004

Ghandourh, W., Dowling, J., Chlap, P., Oar, A., Jacob, S., Batumalai, V., & Holloway, L. (2021). Assessing tumor centrality in lung stereotactic ablative body radiotherapy ( SABR ): the effects of variations in bronchial tree delineation and potential for automated methods. Medical Dosimetry, 46, 94–101. https://doi.org/10.1016/j.meddos.2020.09.004

Also in Cardoso et al https prefix comes twice

samhforbes commented 1 year ago

@editorialbot set version as v0.6.1

editorialbot commented 1 year ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

samhforbes commented 1 year ago

@editorialbot set v0.6.1 as version

editorialbot commented 1 year ago

Done! version is now v0.6.1

samhforbes commented 1 year ago

@editorialbot set 10.5281/zenodo.8032858 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.8032858

samhforbes 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.3389/fninf.2013.00045 is OK
- 10.1007/s10278-017-0037-8 is OK
- 10.5281/ZENODO.7319790 is OK
- 10.1118/1.3468652 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.1371/journal.pone.0245364 is OK
- 10.1016/j.phro.2022.02.011 is OK
- 10.1016/j.meddos.2020.09.004 is OK
- 10.21105/joss.04555 is OK
- 10.1007/s10278-013-9622-7 is OK
- 10.1007/s13246-023-01231-w is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.48550/arXiv.2211.02701 is INVALID because of 'https://doi.org/' prefix
pchlap commented 1 year ago

Hi @pchlap I can see an issue with the bib: Ghandourh, W., Dowling, J., Chlap, P., Oar, A., Jacob, S., Batumalai, V., & Holloway, L. (2021). Medical Dosimetry Assessing tumor centrality in lung stereotactic ablative body radiotherapy ( SABR ): the effects of variations in bronchial tree delineation and potential for automated methods. 46, 94–101. https://doi.org/10.1016/j.meddos.2020.09.004

  • I think should be:

Ghandourh, W., Dowling, J., Chlap, P., Oar, A., Jacob, S., Batumalai, V., & Holloway, L. (2021). Assessing tumor centrality in lung stereotactic ablative body radiotherapy ( SABR ): the effects of variations in bronchial tree delineation and potential for automated methods. Medical Dosimetry, 46, 94–101. https://doi.org/10.1016/j.meddos.2020.09.004

Also in Cardoso et al https prefix comes twice

Thanks @samhforbes, I have corrected these two issues.

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

samhforbes commented 1 year ago

Hi @pchlap unless I'm missing something, Ghandourh et al looks like it still has the Journal name in the article title.

pchlap commented 1 year ago

Ah I'm sorry @samhforbes, I had added it in the journal attribute but failed to remove it from the title!! It should be resolved now though.

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

samhforbes 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.3389/fninf.2013.00045 is OK
- 10.1007/s10278-017-0037-8 is OK
- 10.5281/ZENODO.7319790 is OK
- 10.1118/1.3468652 is OK
- 10.48550/arXiv.2211.02701 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.1371/journal.pone.0245364 is OK
- 10.1016/j.phro.2022.02.011 is OK
- 10.1016/j.meddos.2020.09.004 is OK
- 10.21105/joss.04555 is OK
- 10.1007/s10278-013-9622-7 is OK
- 10.1007/s13246-023-01231-w is OK

MISSING DOIs

- None

INVALID DOIs

- None
samhforbes commented 1 year ago

Hi @pchlap this is all looking really good. I think you've done a great job with software and paper, and I'm going to hand over to the EiC now with a recommendation to accept!

samhforbes 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.3389/fninf.2013.00045 is OK
- 10.1007/s10278-017-0037-8 is OK
- 10.5281/ZENODO.7319790 is OK
- 10.1118/1.3468652 is OK
- 10.48550/arXiv.2211.02701 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.1371/journal.pone.0245364 is OK
- 10.1016/j.phro.2022.02.011 is OK
- 10.1016/j.meddos.2020.09.004 is OK
- 10.21105/joss.04555 is OK
- 10.1007/s10278-013-9622-7 is OK
- 10.1007/s13246-023-01231-w 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/4309, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept