erling6232 / imagedata

Read/write medical image data
MIT License
10 stars 0 forks source link

JOSS Review #3

Closed mwegrzyn closed 2 years ago

mwegrzyn commented 2 years ago

Dear @erling6232,

my name is Martin Wegrzyn and I am one of the reviewers assigned to your submission to JOSS. Thank you for your work on Imagedata. The following are some comments I have regarding my first impression of the package and the manuscript:

installation

Otherwise, I got error messages first regarding scikit-build missing, then regarding problems with cmake, then regarding itk - which I could not solve. If you are interested, I can share the full history of error messages in a separate comment. However, I had these problems on both Linux and Windows and therefore suspect that others might run into similar issues as well. Therefore, I think that specific troubleshooting information, e.g. regarding the itk package, could be helpful.

naming

manuscript

information regarding brain imaging

Documentation

I hope you find these comments to be helpful. Please let me know if you have any questions regarding the points I raised. Thank you for your time and consideration.

Best regards, Martin Wegrzyn

erling6232 commented 2 years ago

Dear @mwegrzyn,

Thank you for your review on our Imagedata software! I will attempt to discuss your specific comments here:

Installation

I'm sorry to hear that there are installation problems. From your description it appears to be ITK-specific problems which is out of scope for this particular project. However, it is in my interest that Imagedata installs without any problems. We have used the software both on Ubuntu Linux, Win10 with Anaconda, and on MacOS.

Please, provide your error log, and your specific hardware and software configuration. Imagedata depends on the itk-io package from PyPI.

Naming

I agree that the name Imagedata is not very specific. There are packages with dicom in their name, suggesting a specific domain. While Imagedata supports dicom, we wanted a name that embraces more data formats. Much like the NiBabel name which suggests a wider usage.

The name has been with us since the inception of Imagedata in 2013 (though the github presence is much shorter.) Renaming the project at this point would require modification of a number of in-house projects.

However, on PyPI, Imagedata is unique!

Manuscript

I'm happy you find the manuscript useful!

The Imagedata software was initially developed for kidney MRI, with DWI and DCE-MRI acquisitions. Another project working on CT and MRI endometrial cancer and artificial intelligence is also making use of this software. At the time there was plenty of software available for brain imaging, and a lack of software for these other organs. So while Imagedata does not exclude brain imaging, the focus has been on providing functionality to Python that were not well covered by NIfTI-based software.

Brain imaging

The different geometrical conventions in DICOM and NIfTI is a problem. While DICOM is scanner-centric, NIfTI geometry is a patient-oriented coordinate system. The general case of converting from one view to the other has not been solved. When you compare dcm2niix to Imagedata, you will find that the NIfTI output is not identical. I do not recommend to mix dcm2niix and Imagedata in the same pipeline. Our focus has been that nifti-dicom-nifti or dicom-nifti-dicom pipelines should restore the original geometry. Therefore, to remove some of the ambiguity in the geometry conversion, we have a DICOM (radiological) view, where left is right.

The NIfTI conversions could be improved with a helping hand! One particularly interesting project is NiTransforms (https://github.com/poldracklab/nitransforms, https://joss.theoj.org/papers/10.21105/joss.03459) which works on spatial transforms.

Inadvertent flipping

The unittest Test3DNIfTIPlugin.test_compare_qform_to_dicom compare the affine transform from the DICOM header to the NIfTI file, where the NIfTI file has been produced using dcm2niix. While the unittest Test3DNIfTIPlugin.test_qform_3D compare the DICOM affine to a NIfTI file produced by Imagedata.

Even if these tests are successful, there can be orientations that are missed in Imagedata.

3D Mosaic

When the functional brain imaging use other pipelines, the 3D Mosaic format has not received any attention here. I agree this would be a useful function, and will look into it. Another 3D format which is not supported is the Enhanced MR and CT DICOM standard. Both of these formats will support 3D volumes in a single file. The Mosaic format would require less modification than the Enhanced formats to include in Imagedata.

Documentation

Thanks for a wise suggestion!

mwegrzyn commented 2 years ago

Dear @erling6232,

thank you for your answers. I will update my review and will let you know about the installation issues in a separate post.

Best, Martin Wegrzyn