Skip to content
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

WIP: add labs script for using pyradiomics with DICOM #437

Merged
merged 12 commits into from Oct 19, 2018

Conversation

fedorov
Copy link
Collaborator

@fedorov fedorov commented Oct 8, 2018

Re #434

@fedorov fedorov requested a review from JoostJM October 8, 2018 18:17
@fedorov
Copy link
Collaborator Author

fedorov commented Oct 8, 2018

@JoostJM I am confused about flake8 errors - local checks pass. Do you know why it behaves differently on CI?

@zhenweishi the updated script is here. Once this PR is merged, it will be ready for your use.

@fedorov
Copy link
Collaborator Author

fedorov commented Oct 8, 2018

TODO:

  • include pyradiomics version
  • fix flake8 issues
  • propagate laterality when available in SEG (not critical for this PR, since none of the segmentations for the @zhenweishi project will have modifiers for segmentations)
  • use Algorithm identification to store params file (depends on Improve support for algorithm identification QIICR/dcmqi#361)
  • revisit the mapping table for the IBSI codes (also cross-check with CP-1764: ftp://medical.nema.org/medical/dicom/cp/cp1764_04_moreimagefeatures.pdf)

@fedorov
Copy link
Collaborator Author

fedorov commented Oct 9, 2018

I am confused about flake8 errors - local checks pass. Do you know why it behaves differently on CI?

I think I understand - I did not take into account the project specific settings in https://github.com/Radiomics/pyradiomics/blob/2b98e8b15a2a8a3574869cb82dc32473591a2e09/.flake8

@JoostJM
Copy link
Collaborator

JoostJM commented Oct 10, 2018

I think I understand - I did not take into account the project specific settings in

Should not matter if you run flake8 from the root directory.

"modifierValue": self.makePrivateCode("Exponent transformation")})

# parameterized processing operations
elif re.match("wavelet-([HL]{3})", prefix):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should allow 2 characters in case of forced 2d extraction.

Copy link
Collaborator

@JoostJM JoostJM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I just pushed a last commit with fixes of some typos, and mainly updated the coding style of the script to match that of the rest of PyRadiomics.

@zhenweishi
Copy link

zhenweishi commented Oct 11, 2018 via email

@JoostJM
Copy link
Collaborator

JoostJM commented Oct 11, 2018

btw @fedorov, I did a forced-push to remove an unnecessary merge from the history.

@fedorov
Copy link
Collaborator Author

fedorov commented Oct 11, 2018

Thank you @JoostJM! I will make a new release of dcmqi so it is easier to reference, and then will update the version pointer and merge. Sorry about the "dirty" commit history, thank you for fixing that.

@fedorov
Copy link
Collaborator Author

fedorov commented Oct 11, 2018

I may need to do few adjustments. I found a meaningful place to put pyradiomics processing parameters, but I will need to add things to dcmqi first to allow encoding of those attributes.

@fedorov
Copy link
Collaborator Author

fedorov commented Oct 15, 2018

@zhenweishi can you tell me when you are planning to do the conversion? There is this one issue about encoding algorithm identification that I would like to fix, and it would help me prioritize things if I know your plans a bit better.

fedorov and others added 12 commits October 19, 2018 16:52
Mapping of features to ISBI is added temporarily, pending resolution
of #435
* added README
* added sample command line and parameter file that will be use in the @zhenweishi study
Update import order to adhere to coding style (prevents flake8 errors)

Additionally, remove usage of pathlib, as it is not available in python 2.7 (It is only part of the std lib since 3.4).
 * recognize wavelet features computed with forced 2d
 * do not require both plastimatch and dcm2niix installed
Update style to follow PyRadiomics Style (2 space indent, logger child of 'radiomics' logger).
Additionally, add some minor changes (e.g. use of choices in argument parser, mark methods as static where applicable)
Depends on the functionality added to dcmqi in this commit:

QIICR/dcmqi@ef688c4
This corresponds to the v7 of the IBSI document, and was used as a basis for
generating featureDict.tsv in resources.
Use updated names of the pyradiomics features, and map them to the IBSI
concepts more precisely
The CP is using different (more precise) than in IBSI feature names for some of them.

In the future, we could add tools to automatically extract feature nomenclature from
the IBSI TeX file and from the DICOM standard, and automatically assemble the mapping
table. Added here for the reference.

File provided by David Clunie.
@fedorov fedorov merged commit 0dc8bbb into master Oct 19, 2018
@fedorov fedorov deleted the add-pyradiomics-dcm branch February 1, 2019 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants