Reading DICOM SR and creation of JSON with its attributes and measurements #110

che85 · 2016-11-04T02:15:30Z

fixes #88
fixes #109

I changed the cli code for jsoncompare in order to add a list of ignored keys from cmake:

['activitySession', 'timePoint', 'imageLibrary', 'compositeContext']

fedorov · 2016-11-04T02:25:57Z

Thank you! I will review tomorrow or weekend. Tomorrow is going to be a bit crazy...

che85 · 2016-11-04T02:27:43Z

That's fine. I am going to look into the build errors.... Maybe it makes sense to update the dcmtk

fedorov · 2016-11-04T02:28:41Z

Yes, definitely makes sense.

che85 · 2016-11-04T05:12:48Z

@fedorov Should work now.

fedorov · 2016-11-04T16:51:43Z

apps/sr/tid1500reader.cxx

+                            j["codeMeaning"].asCString());
+}
+
+void addFileToEvidence(DSRDocument &doc, string dirStr, string fileStr){


this function is not used - remove?

fedorov · 2016-11-04T16:56:08Z

apps/sr/tid1500reader.cxx

+        if (st.gotoNamedChildNode(CODE_DCM_Finding)) {
+          measurement["Finding"]["CodeMeaning"] = st.getCurrentContentItem().getCodeValue().getCodeMeaning().c_str();
+          measurement["Finding"]["CodingSchemeDesignator"] = st.getCurrentContentItem().getCodeValue().getCodingSchemeDesignator().c_str();
+          measurement["Finding"]["CodeValue"] = st.getCurrentContentItem().getCodeValue().getCodeValue().c_str();


I am shocked you didn't abstract these 3 lines into a function - you are picking bad habits from me!! ;-)

Haha I thought for now (for the RSNA) it's fine.... Refactoring later.... I first want to get everything work....

fedorov · 2016-11-04T17:19:44Z

apps/sr/tid1500reader.cxx

+        st.gotoChild();
+
+        Json::Value measurementItems(Json::arrayValue);
+        while (st.gotoNext()){


Can you test it with this dataset: https://fedorov.gitbooks.io/rsna2016-qirr-dicom4qi/content/instructions/sr-tid1500.html#test-dataset-2

I've just added it to RSNA DICOM4QI. It contains large number of measurement groups and measurements. Should be a good challenge for testing ...

@fedorov How do you want me to test that? Just generating JSON from that DICOM SR (http://slicer.kitware.com/midas3/download/item/262094/Measurements_User2_SemiAuto_Trial2.dcm)?

And how do I know, if it's correct? There is no json to compare it to, right?

Yes, just generate JSON, and then compare it to the output of dsrdump (or use Atom dicom-dump plugin that exposes dsrdump).

Yes, that's a good test. Having problems if there is more than 1 MeasurementGroup. Need to check that....

But I know what the problem is. Just need to find out, how to fix that...

Ok I fixed that, but I am not reading rwvm from SR. Should I also implement those things? As well as MeasurementMethod is missing in the json output

fedorov · 2016-11-04T17:29:17Z

apps/sr/tid1500reader.cxx

+  cout << "Number of verifying observers: " << doc.getNumberOfVerifyingObservers() << endl;
+
+  OFString observerName, observingDateTime, organizationName;
+  if (doc.getNumberOfVerifyingObservers() != 0) {


I did not realize this, but now that I looked - it seems that there are two places where observer is recorded. The one I used for writing TID1500 is TID1001 http://dicom.nema.org/medical/dicom/current/output/chtml/part16/chapter_A.html#sect_TID_1001 Observation context: https://github.com/QIICR/dcmqi/blob/master/apps/sr/tid1500writer.cxx#L78-L84

I agree that in the future we should synchronize this with DCM_Verifying/Verification* items, but for now I think it will be better if reader is symmetric with the writer.

fedorov · 2016-11-04T17:32:17Z

apps/sr/tid1500writer.cxx

+    if (DSRTypes::enumeratedValueToVerificationFlag(metaRoot["VerificationFlag"].asCString()) ==
+        DSRTypes::VF_Verified) {
+      // TODO: get organization from meta information?
+      CHECK_COND(doc.verifyDocument(metaRoot["observerContext"]["PersonObserverName"].asCString(), "BWH"));


let's use QIICR in place of BWH for consistency

fedorov · 2016-11-04T17:33:10Z

libsrc/ImageSEGConverter.cpp

-      segdoc->getSeries().setSeriesDate(contentDate.c_str());
-      segdoc->getSeries().setSeriesTime(contentTime.c_str());
+      CHECK_COND(segdocDataset.putAndInsertString(DCM_SeriesDate, contentDate.c_str()));
+      CHECK_COND(segdocDataset.putAndInsertString(DCM_SeriesTime, contentTime.c_str()));


Did you test with the newest dcmtk - is this still a problem with using getSeries()?

No I didn't try that, but will check this locally

fedorov · 2016-11-04T20:49:20Z

Ok I fixed that, but I am not reading rwvm from SR. Should I also
implement those things? As well as MeasurementMethod is missing in the json
output

@che85 this is fine, let's not worry about this in the initial
implementation.

…y) from DICOM SR (issue QIICR#88)

…te SR into JSON (issue QIICR#88)

- also added testing

- added roundtrip test for meta information and added ignored keys for those we know are different: - ['activitySession', 'timePoint', 'imageLibrary', 'compositeContext'] - added additional argument for compare for ignoredKeys

…ataset

…sue QIICR#88)

che85 · 2016-11-07T23:05:07Z

@fedorov The job exceeded the maximum time limit for jobs, and has been terminated.

fedorov · 2016-11-07T23:10:09Z

hm, unpleasant news ... time to package DCMTK and ITK for Mac?

che85 · 2016-11-08T00:36:07Z

@fedorov I just restarted the build 😄

fedorov · 2016-11-08T18:18:04Z

@che85 is this all set or you are still working on it?

fedorov · 2016-11-08T18:19:05Z

I just restarted the build 😄

we have 10 min buffer for now! 50 minutes is the maximum allowed on TravisCI

che85 · 2016-11-08T18:22:14Z

Yes should be ready for merge....

fedorov · 2016-11-08T21:10:53Z

👍

fedorov requested changes Nov 4, 2016

View reviewed changes

jcfr mentioned this pull request Nov 4, 2016

Update build system #111

Merged

che85 added 11 commits November 7, 2016 12:26

ENH: successfully reading evidence images (composite and image librar…

f3b786f

…y) from DICOM SR (issue QIICR#88)

ENH: Only measurementItems missing and then it should read the comple…

ed03cde

…te SR into JSON (issue QIICR#88)

ENH: successfully reading measurements from DICOM SR (issue QIICR#88)

fcb964e

- also added testing

ENH: Added missing attributes (issue QIICR#109)

bc2880f

ENH: Updated DCMTK revision

bfc4d14

ENH: inserting seriesData and SeriesTime directly into segmentation d…

bf26b8c

…ataset

BUG: included missing files

9837935

ENH: Removed unneeded include and added others

2ac8f7c

ENH: refactored code and removed unused functions

f8f5cb8

ENH: Reading n MeasurementGroups from DICOM SR into json possible (is…

3c7405e

…sue QIICR#88)

che85 force-pushed the TID15002JSON branch from 442d11f to 3c7405e Compare November 7, 2016 19:34

ENH: cleaning up cmake

afdef2e

fedorov merged commit 9022e5a into QIICR:master Nov 8, 2016

che85 deleted the TID15002JSON branch November 18, 2016 07:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reading DICOM SR and creation of JSON with its attributes and measurements #110

Reading DICOM SR and creation of JSON with its attributes and measurements #110

che85 commented Nov 4, 2016

fedorov commented Nov 4, 2016

che85 commented Nov 4, 2016

fedorov commented Nov 4, 2016

che85 commented Nov 4, 2016

fedorov Nov 4, 2016

fedorov Nov 4, 2016

che85 Nov 4, 2016

fedorov Nov 4, 2016

che85 Nov 4, 2016

fedorov Nov 4, 2016

che85 Nov 4, 2016

che85 Nov 4, 2016

che85 Nov 4, 2016

fedorov Nov 4, 2016

fedorov Nov 4, 2016

fedorov Nov 4, 2016

che85 Nov 4, 2016

fedorov commented Nov 4, 2016

che85 commented Nov 7, 2016

fedorov commented Nov 7, 2016

che85 commented Nov 8, 2016

fedorov commented Nov 8, 2016

fedorov commented Nov 8, 2016

che85 commented Nov 8, 2016

fedorov commented Nov 8, 2016

Reading DICOM SR and creation of JSON with its attributes and measurements #110

Reading DICOM SR and creation of JSON with its attributes and measurements #110

Conversation

che85 commented Nov 4, 2016

fedorov commented Nov 4, 2016

che85 commented Nov 4, 2016

fedorov commented Nov 4, 2016

che85 commented Nov 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedorov commented Nov 4, 2016

che85 commented Nov 7, 2016

fedorov commented Nov 7, 2016

che85 commented Nov 8, 2016

fedorov commented Nov 8, 2016

fedorov commented Nov 8, 2016

che85 commented Nov 8, 2016

fedorov commented Nov 8, 2016