New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function that loads MetaImage files #119

Merged
merged 1 commit into from Sep 19, 2017

Conversation

Projects
None yet
4 participants
@vessemer
Copy link
Contributor

vessemer commented Sep 18, 2017

The load_metaimage function that load MetaImage files have been proposed along with load_ct ensembling method which aimed to organize loading process. Refactoring has been performed, taking into account various format of meta-information. In order to handle it standardisation via class MetaData has been employed, for now, it contains only one parameter (spacing) which is used already. Also decoupling of reading and preprocessing was included.

Motivation and Context

This addresses #98. The MetaImage file format is actively used by plenty of medical studies. The LUNA16 challenge is among them. Since LUNA16 was involved in kaggle DSB 2017 competition, therefore, it's into of our area of interest.

How Has This Been Tested?

The unit tests were created for each of introduced functions.

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
@tdraebing

This comment has been minimized.

Copy link
Contributor

tdraebing commented Sep 18, 2017

Hi @vessemer,

thanks for the pull request. I am currently pretty swamped with the last months in my PhD, so I didn't manage to finish my pull request in the last days.
Based on my work on the issue, I have some further suggestions:

  • The load_dicom-function uses pydicom and dicom-numpy to load the DICOM-images. SimpleITK also has a method implemented to do so (which might even be more tolerant to compressed images). Fully switching to SimpleITK would reduce the number of dependencies and make the code easier to maintain and follow. There would be also some changes in crop_dicom and preprocess_dicom needed. This might also be handled in a new issue.
  • In your MetaData-class you calculate the slice thickness. There actually is a value for this in the metadata of the DICOM-images: '0018|0050'. (or for a pydicom-object SliceThickness.
  • The load_meta-function seems to be quite redundant to load_ct. Maybe by adding a flag (like voxel=True if the voxel data should be returned as well) would be better here.
  • This is just my personal preference, but maybe it would make sense to put the logic in MetaData.__init__() into a separate method(s). That would make it easier to extend the object later on. For example if patient data becomes relevant in the interface, which would also be stored in the metadata.
  • From your tests I read that you added data from the LUNA16-competition. Did you check, whether using them in other projects is fine with their license?
@dchansen

This comment has been minimized.

Copy link
Contributor

dchansen commented Sep 18, 2017

I would strongly support moving to SimpleITK for everything, as it correctly deals with things like calibration curves, orientations, etc and gives us an easy way to pass along images and their metadata in one package.

@vessemer vessemer force-pushed the vessemer:98_metaimage_and_preprocessing branch from 77b770d to 950430f Sep 18, 2017

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Sep 18, 2017

Hi @tdraebing, thanks for your detailed review and suggestions. I've refactored the code, all rebased and stashed.

From your tests I read that you added data from the LUNA16-competition. Did you check, whether using them in other projects is fine with their license?

As it was claimed on the official page of LUNA16/data the dataset was made from LIDC/IDRI database via filtering out CT scans which slice thickness greater than 2.5 mm and may be other processing. The LIDC/IDRI database, in turn, consist of DICOM files and have Creative Commons Attribution 3.0 Unported License. Thus it's fine to use the LUNA16 data in a project.

Fully switching to SimpleITK would reduce the number of dependencies and make the code easier to maintain and follow. There would be also some changes in crop_dicom and preprocess_dicom needed. This might also be handled in a new issue.

I guess it will be a good idea to open a new issue dedicated to that task since it will lead to a quite cumbersome refactoring. @tdraebing, would you like to open it?

@vessemer vessemer force-pushed the vessemer:98_metaimage_and_preprocessing branch 3 times, most recently from 0e0cd79 to e3ed1b8 Sep 18, 2017

@tdraebing tdraebing referenced this pull request Sep 18, 2017

Closed

Use SimpleITK to load DICOM-images #121

0 of 3 tasks complete
@tdraebing

This comment has been minimized.

Copy link
Contributor

tdraebing commented Sep 18, 2017

Awesome. Thanks for checking up on the license! I opened a new issue to deal with loading the DICOM-images with SimpleITK.

@vessemer vessemer force-pushed the vessemer:98_metaimage_and_preprocessing branch from e3ed1b8 to 263699b Sep 18, 2017

@lamby lamby merged commit 91cbff4 into drivendataorg:master Sep 19, 2017

2 checks passed

concept-to-clinic/cla @vessemer has signed the CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Sep 19, 2017

Great stuff guys! Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment