Skip to content

Commit fcf672f

Browse files
committed
BUG: fixed segmentation round trip test (issue QIICR#9)
- added missing attributes - fixed json comparison script (for the reference ChannelIQ/jsoncompare#8 and ChannelIQ/jsoncompare#9)
1 parent 74ebdae commit fcf672f

7 files changed

Lines changed: 102 additions & 98 deletions

apps/seg/Testing/CMakeLists.txt

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,48 @@
1-
set(MODULE_NAME segimage2itkimage)
2-
3-
add_test(NAME ${MODULE_NAME}_hello
4-
COMMAND ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${MODULE_NAME} --help)
5-
1+
set(MODULE_NAME segmentation)
62
set(BASELINE ${CMAKE_SOURCE_DIR}/data/ct-3slice/)
73

84
set(MODULE_TEMP_DIR ${TEMP_DIR}/seg)
95
make_directory(${MODULE_TEMP_DIR})
106

7+
set(itk2dcm itkimage2segimage)
8+
9+
add_test(NAME ${itk2dcm}_hello
10+
COMMAND ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${itk2dcm} --help)
11+
set_property(TEST ${itk2dcm}_hello PROPERTY LABELS ${MODULE_NAME})
12+
13+
add_test(NAME ${itk2dcm}_makeSEG
14+
COMMAND ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${itk2dcm}
15+
--metaDataFileName ${CMAKE_SOURCE_DIR}/doc/seg-example.json
16+
--segImageFiles ${BASELINE}/liver_seg.nrrd
17+
--dicomImageFiles ${BASELINE}/01.dcm,${BASELINE}/02.dcm,${BASELINE}/03.dcm
18+
--segDICOMFile ${MODULE_TEMP_DIR}/liver.dcm)
19+
set_property(TEST ${itk2dcm}_makeSEG PROPERTY LABELS ${MODULE_NAME})
20+
21+
set(dcm2itk segimage2itkimage)
22+
23+
add_test(NAME ${dcm2itk}_hello
24+
COMMAND ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${dcm2itk} --help)
25+
set_property(TEST ${dcm2itk}_hello PROPERTY LABELS ${MODULE_NAME})
26+
1127
include_directories(${ITK_INCLUDE_DIRS})
12-
add_executable(${MODULE_NAME}Test ${MODULE_NAME}Test.cxx )
13-
target_link_libraries(${MODULE_NAME}Test ${${MODULE_NAME}_TARGET_LIBRARIES})
14-
set_target_properties(${MODULE_NAME}Test PROPERTIES LABELS ${MODULE_NAME})
15-
set_target_properties(${MODULE_NAME}Test PROPERTIES FOLDER ${${MODULE_NAME}_TARGETS_FOLDER})
16-
17-
add_test(NAME ${MODULE_NAME}_makeNRRD COMMAND ${SEM_LAUNCH_COMMAND} $<TARGET_FILE:${MODULE_NAME}Test>
18-
--compare ${BASELINE}/liver_seg.nrrd
19-
${MODULE_TEMP_DIR}/1.nrrd
20-
${MODULE_NAME}Test
21-
${BASELINE}/liver.dcm
22-
${MODULE_TEMP_DIR})
23-
24-
set_property(TEST ${testname} PROPERTY LABELS ${MODULE_NAME})
25-
26-
set(MODULE_NAME itkimage2segimage)
27-
28-
add_test(NAME ${MODULE_NAME}_hello
29-
COMMAND ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${MODULE_NAME} --help)
30-
add_test(NAME ${MODULE_NAME}_makeSEG
31-
COMMAND ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${MODULE_NAME}
32-
--metaDataFileName ${CMAKE_SOURCE_DIR}/doc/seg-example.json
33-
--segImageFiles ${BASELINE}/liver_seg.nrrd
34-
--dicomImageFiles ${BASELINE}/01.dcm,${BASELINE}/02.dcm,${BASELINE}/03.dcm
35-
--segDICOMFile ${MODULE_TEMP_DIR}/liver.dcm
36-
)
28+
add_executable(${dcm2itk}Test ${dcm2itk}Test.cxx )
29+
target_link_libraries(${dcm2itk}Test ${${dcm2itk}_TARGET_LIBRARIES})
30+
set_target_properties(${dcm2itk}Test PROPERTIES LABELS ${MODULE_NAME})
31+
set_target_properties(${dcm2itk}Test PROPERTIES FOLDER ${${dcm2itk}_TARGETS_FOLDER})
32+
33+
add_test(NAME ${dcm2itk}_makeNRRD COMMAND ${SEM_LAUNCH_COMMAND} $<TARGET_FILE:${dcm2itk}Test>
34+
--compare ${BASELINE}/liver_seg.nrrd
35+
${MODULE_TEMP_DIR}/1.nrrd
36+
${dcm2itk}Test
37+
${MODULE_TEMP_DIR}/liver.dcm
38+
${MODULE_TEMP_DIR})
39+
set_tests_properties(${dcm2itk}_makeNRRD PROPERTIES DEPENDS ${itk2dcm}_makeSEG)
40+
set_property(TEST ${dcm2itk}_makeNRRD PROPERTY LABELS ${MODULE_NAME})
41+
3742

3843
add_test(NAME seg_meta_roundtrip
39-
COMMAND python ${CMAKE_SOURCE_DIR}/util/comparejson.py
40-
${MODULE_TEMP_DIR}/meta.json
41-
${CMAKE_SOURCE_DIR}/doc/seg-example.json
42-
)
44+
COMMAND python ${CMAKE_SOURCE_DIR}/util/comparejson.py
45+
${CMAKE_SOURCE_DIR}/doc/seg-example.json
46+
${MODULE_TEMP_DIR}/meta.json)
47+
set_tests_properties(seg_meta_roundtrip PROPERTIES DEPENDS ${dcm2itk}_makeNRRD)
48+
set_property(TEST seg_meta_roundtrip PROPERTY LABELS ${MODULE_NAME})

include/JSONSegmentationMetaInformationHandler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ namespace dcmqi {
1616
~JSONSegmentationMetaInformationHandler();
1717

1818
void setContentCreatorName(const string &creatorName);
19+
void setClinicalTrialCoordinatingCenterName(const string &coordinatingCenterName);
1920
void setClinicalTrialSeriesID(const string &seriesID);
2021
void setClinicalTrialTimePointID(const string &timePointID);
2122

2223
string getContentCreatorName() const { return contentCreatorName; }
24+
string getClinicalTrialCoordinatingCenterName() const { return coordinatingCenterName; }
2325
string getClinicalTrialSeriesID() const { return clinicalTrialSeriesID; }
2426
string getClinicalTrialTimePointID() const { return clinicalTrialTimePointID; }
2527

@@ -37,6 +39,7 @@ namespace dcmqi {
3739
protected:
3840

3941
string contentCreatorName;
42+
string coordinatingCenterName;
4043
string clinicalTrialSeriesID;
4144
string clinicalTrialTimePointID;
4245

libsrc/ImageSEGConverter.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -375,12 +375,12 @@ namespace dcmqi {
375375
CHECK_COND(segdoc->writeDataset(segdocDataset));
376376

377377
// Set reader/session/timepoint information
378+
CHECK_COND(segdocDataset.putAndInsertString(DCM_SeriesDescription, metaInfo.getSeriesDescription().c_str()));
378379
CHECK_COND(segdocDataset.putAndInsertString(DCM_ContentCreatorName, metaInfo.getContentCreatorName().c_str()));
379380
CHECK_COND(segdocDataset.putAndInsertString(DCM_ClinicalTrialSeriesID, metaInfo.getClinicalTrialSeriesID().c_str()));
380381
CHECK_COND(segdocDataset.putAndInsertString(DCM_ClinicalTrialTimePointID, metaInfo.getClinicalTrialTimePointID().c_str()));
381-
if(metaInfo.metaInfoRoot["seriesAttributes"].isMember("ClinicalTrialCoordinatingCenterName"))
382-
CHECK_COND(segdocDataset.putAndInsertString(DCM_ClinicalTrialCoordinatingCenterName,
383-
metaInfo.metaInfoRoot["seriesAttributes"]["ClinicalTrialCoordinatingCenterName"].asCString()));
382+
if (metaInfo.getClinicalTrialCoordinatingCenterName().size())
383+
CHECK_COND(segdocDataset.putAndInsertString(DCM_ClinicalTrialCoordinatingCenterName, metaInfo.getClinicalTrialCoordinatingCenterName().c_str()));
384384

385385
// populate BodyPartExamined
386386
{
@@ -408,9 +408,6 @@ namespace dcmqi {
408408
segdoc->getSeries().setSeriesTime(contentTime.c_str());
409409
segdoc->getGeneralImage().setContentDate(contentDate.c_str());
410410
segdoc->getGeneralImage().setContentTime(contentTime.c_str());
411-
412-
segdoc->getSeries().setSeriesDescription(metaInfo.getSeriesDescription().c_str());
413-
segdoc->getSeries().setSeriesNumber(metaInfo.getSeriesNumber().c_str());
414411
}
415412

416413
return new DcmDataset(segdocDataset);
@@ -697,20 +694,22 @@ namespace dcmqi {
697694

698695
void ImageSEGConverter::populateMetaInformationFromDICOM(DcmDataset *segDataset, DcmSegmentation *segdoc,
699696
JSONSegmentationMetaInformationHandler &metaInfo) {
700-
OFString readerID, sessionID, timePointID, seriesDescription, seriesNumber, instanceNumber, bodyPartExamined;
697+
OFString creatorName, sessionID, timePointID, seriesDescription, seriesNumber, instanceNumber, bodyPartExamined, coordinatingCenter;
701698

702-
segdoc->getContentIdentification().getInstanceNumber(instanceNumber);
703-
segdoc->getContentIdentification().getContentCreatorName(readerID);
699+
segDataset->findAndGetOFString(DCM_InstanceNumber, instanceNumber);
700+
segdoc->getContentIdentification().getContentCreatorName(creatorName);
704701

705702
segDataset->findAndGetOFString(DCM_ClinicalTrialTimePointID, timePointID);
706703
segDataset->findAndGetOFString(DCM_ClinicalTrialSeriesID, sessionID);
704+
segDataset->findAndGetOFString(DCM_ClinicalTrialCoordinatingCenterName, coordinatingCenter);
707705

708706
segdoc->getSeries().getBodyPartExamined(bodyPartExamined);
709707
segdoc->getSeries().getSeriesNumber(seriesNumber);
710708
segdoc->getSeries().getSeriesDescription(seriesDescription);
711709

712-
metaInfo.setContentCreatorName(readerID.c_str());
713-
metaInfo.setClinicalTrialTimePointID(sessionID.c_str());
710+
metaInfo.setClinicalTrialCoordinatingCenterName(coordinatingCenter.c_str());
711+
metaInfo.setContentCreatorName(creatorName.c_str());
712+
metaInfo.setClinicalTrialSeriesID(sessionID.c_str());
714713
metaInfo.setSeriesNumber(seriesNumber.c_str());
715714
metaInfo.setClinicalTrialTimePointID(timePointID.c_str());
716715
metaInfo.setSeriesDescription(seriesDescription.c_str());

libsrc/JSONMetaInformationHandlerBase.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ namespace dcmqi {
2929

3030
Json::Value JSONMetaInformationHandlerBase::codeSequence2Json(CodeSequenceMacro *codeSequence) {
3131
Json::Value value;
32-
value["codeValue"] = getCodeSequenceValue(codeSequence);
33-
value["codingSchemeDesignator"] = getCodeSequenceDesignator(codeSequence);
34-
value["codeMeaning"] = getCodeSequenceMeaning(codeSequence);
32+
value["CodeValue"] = getCodeSequenceValue(codeSequence);
33+
value["CodingSchemeDesignator"] = getCodeSequenceDesignator(codeSequence);
34+
value["CodeMeaning"] = getCodeSequenceMeaning(codeSequence);
3535
return value;
3636
}
3737

libsrc/JSONSegmentationMetaInformationHandler.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ namespace dcmqi {
2020
}
2121
}
2222

23+
void JSONSegmentationMetaInformationHandler::setClinicalTrialCoordinatingCenterName(const string &coordinatingCenterName) {
24+
this->coordinatingCenterName = coordinatingCenterName;
25+
}
26+
2327
void JSONSegmentationMetaInformationHandler::setContentCreatorName(const string &creatorName) {
2428
this->contentCreatorName = creatorName;
2529
}
@@ -72,8 +76,9 @@ namespace dcmqi {
7276
void JSONSegmentationMetaInformationHandler::readSeriesAttributes() {
7377
Json::Value seriesAttributes = this->metaInfoRoot["seriesAttributes"];
7478
this->contentCreatorName = seriesAttributes.get("ContentCreatorName", "Reader1").asString();
79+
this->coordinatingCenterName = seriesAttributes.get("ClinicalTrialCoordinatingCenterName", "").asString();
7580
this->clinicalTrialSeriesID = seriesAttributes.get("SessionID", "Session1").asString();
76-
this->clinicalTrialTimePointID = seriesAttributes.get("TimePointID", "1").asString();
81+
this->clinicalTrialTimePointID = seriesAttributes.get("ClinicalTrialTimePointID", "1").asString();
7782
this->seriesDescription = seriesAttributes.get("SeriesDescription", "Segmentation").asString();
7883
this->seriesNumber = seriesAttributes.get("SeriesNumber", "300").asString();
7984
this->instanceNumber = seriesAttributes.get("InstanceNumber", "1").asString();
@@ -83,8 +88,10 @@ namespace dcmqi {
8388
Json::Value JSONSegmentationMetaInformationHandler::createAndGetSeriesAttributes() {
8489
Json::Value value;
8590
value["ContentCreatorName"] = this->contentCreatorName;
91+
if (this->coordinatingCenterName.size())
92+
value["ClinicalTrialCoordinatingCenterName"] = this->coordinatingCenterName;
8693
value["ClinicalTrialSeriesID"] = this->clinicalTrialSeriesID;
87-
value["TimePointID"] = this->clinicalTrialTimePointID;
94+
value["ClinicalTrialTimePointID"] = this->clinicalTrialTimePointID;
8895
value["SeriesDescription"] = this->seriesDescription;
8996
value["SeriesNumber"] = this->seriesNumber;
9097
value["InstanceNumber"] = this->instanceNumber;

util/comparejson.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
json1 = json.loads(open(sys.argv[1],'r').read())
77
json2 = json.loads(open(sys.argv[2],'r').read())
88

9-
(check,stack) = jsoncompare.are_same(json1,json2,True,True,["@schema"])
9+
(check,stack) = jsoncompare.are_same(json1,json2,
10+
ignore_list_order_recursively=True,
11+
ignore_missing_keys=True,
12+
ignore_value_of_keys=["@schema"])
1013

1114
if not check:
1215
print stack

util/jsoncompare.py

Lines changed: 33 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import json
2-
from pprint import pprint
2+
33

44
class Stack:
55
def __init__(self):
@@ -52,23 +52,23 @@ def _generate_pprint_json(value):
5252
def _is_dict_same(expected, actual, ignore_value_of_keys):
5353
# DAN - I had to flip flop this
5454
for key in expected:
55-
if not key in actual:
55+
if key in ignore_value_of_keys:
56+
continue
57+
if key not in actual:
5658
return False, \
5759
Stack().append(
5860
StackItem('Expected key "{0}" Missing from Actual'
5961
.format(key),
6062
expected,
6163
actual))
6264

63-
if not key in ignore_value_of_keys:
64-
# have to change order
65-
#are_same_flag, stack = _are_same(actual[key], expected[key], ignore_value_of_keys)
66-
are_same_flag, stack = _are_same(expected[key], actual[key],ignore_value_of_keys)
67-
if not are_same_flag:
68-
return False, \
69-
stack.append(StackItem('Different values', expected[key], actual[key]))
65+
are_same_flag, stack = _are_same(expected[key], actual[key],ignore_value_of_keys)
66+
if not are_same_flag:
67+
return False, \
68+
stack.append(StackItem('Different values', expected[key], actual[key]))
7069
return True, Stack()
7170

71+
7272
def _is_list_same(expected, actual, ignore_value_of_keys):
7373
for i in xrange(len(expected)):
7474
are_same_flag, stack = _are_same(expected[i], actual[i], ignore_value_of_keys)
@@ -78,6 +78,7 @@ def _is_list_same(expected, actual, ignore_value_of_keys):
7878
StackItem('Different values (Check order)', expected[i], actual[i]))
7979
return True, Stack()
8080

81+
8182
def _bottom_up_sort(unsorted_json):
8283
if isinstance(unsorted_json, list):
8384
new_list = []
@@ -94,6 +95,7 @@ def _bottom_up_sort(unsorted_json):
9495
else:
9596
return unsorted_json
9697

98+
9799
def _are_same(expected, actual, ignore_value_of_keys, ignore_missing_keys=False):
98100
# Check for None
99101
if expected is None:
@@ -112,29 +114,12 @@ def _are_same(expected, actual, ignore_value_of_keys, ignore_missing_keys=False)
112114
if type(expected) in (int, str, bool, long, float, unicode):
113115
return expected == actual, Stack()
114116

115-
# Ensure collections have the same length (if applicable)
116-
if ignore_missing_keys:
117-
# Ensure collections has minimum length (if applicable)
118-
# This is a short-circuit condition because (b contains a)
119-
if len(expected) > len(actual):
120-
return False, \
121-
Stack().append(
122-
StackItem('Length Mismatch: Minimum Expected Length: {0}, Actual Length: {1}'
123-
.format(len(expected), len(actual)),
124-
expected,
125-
actual))
126-
127-
else:
128-
# Ensure collections has same length
129-
if len(expected) != len(actual):
130-
return False, \
131-
Stack().append(
132-
StackItem('Length Mismatch: Expected Length: {0}, Actual Length: {1}'
133-
.format(len(expected), len(actual)),
134-
expected,
135-
actual))
136-
137-
117+
if not ignore_missing_keys and len(expected) > len(actual):
118+
stack = Stack().append(StackItem('Length Mismatch: Expected Length: {0}, Actual Length: {1}'
119+
.format(len(expected), len(actual)), expected, actual))
120+
if isinstance(expected, dict):
121+
stack.append('Missing keys: {0}'.format(get_missing_keys(expected, actual)))
122+
return False, stack
138123

139124
if isinstance(expected, dict):
140125
return _is_dict_same(expected, actual, ignore_value_of_keys)
@@ -144,25 +129,26 @@ def _are_same(expected, actual, ignore_value_of_keys, ignore_missing_keys=False)
144129

145130
return False, Stack().append(StackItem('Unhandled Type: {0}'.format(type(expected)), expected, actual))
146131

147-
def are_same(original_a, original_b, ignore_list_order_recursively=False, ignore_missing_keys=False, ignore_value_of_keys=[]):
148-
if ignore_list_order_recursively:
149-
a = _bottom_up_sort(original_a)
150-
b = _bottom_up_sort(original_b)
151-
else:
152-
a = original_a
153-
b = original_b
132+
133+
def get_missing_keys(expected, actual):
134+
return [key for key in expected if key not in actual]
135+
136+
137+
def are_same(original_a, original_b, ignore_list_order_recursively=False, ignore_missing_keys=False, ignore_value_of_keys=None):
138+
ignore_value_of_keys = ignore_value_of_keys if ignore_value_of_keys else []
139+
a = _bottom_up_sort(original_a) if ignore_list_order_recursively else original_a
140+
b = _bottom_up_sort(original_b) if ignore_list_order_recursively else original_b
154141
return _are_same(a, b, ignore_value_of_keys, ignore_missing_keys)
155142

156143

157-
def contains(expected_original, actual_original, ignore_list_order_recursively=False, ignore_value_of_keys=[]):
158-
if ignore_list_order_recursively:
159-
actual = _bottom_up_sort(actual_original)
160-
expected = _bottom_up_sort(expected_original)
161-
else:
162-
actual = actual_original
163-
expected = expected_original
144+
def contains(expected_original, actual_original, ignore_list_order_recursively=False, ignore_value_of_keys=None):
145+
ignore_value_of_keys = ignore_value_of_keys if ignore_value_of_keys else []
146+
actual = _bottom_up_sort(actual_original) if ignore_list_order_recursively else actual_original
147+
expected = _bottom_up_sort(expected_original) if ignore_list_order_recursively else expected_original
164148
return _are_same(expected, actual, ignore_value_of_keys, True)
165149

166-
def json_are_same(a, b, ignore_list_order_recursively=False, ignore_value_of_keys=[]):
150+
151+
def json_are_same(a, b, ignore_list_order_recursively=False, ignore_value_of_keys=None):
152+
ignore_value_of_keys = ignore_value_of_keys if ignore_value_of_keys else []
167153
return are_same(json.loads(a), json.loads(b), ignore_list_order_recursively, ignore_value_of_keys)
168154

0 commit comments

Comments
 (0)