QIICR / QuantitativeReporting

Segmentation-based measurements with DICOM import and export of the results.
https://qiicr.gitbooks.io/quantitativereporting-guide
Other
23 stars 23 forks source link

SegmentedPropertyTypeModifier seems to be ignored while loading terminology from JSON #162

Closed fedorov closed 7 years ago

fedorov commented 7 years ago

SegmentedPropertyTypeModifier is not showing up in the GUI, although it looks like it is present in the JSON file produced by the dcmqi converter.

Test dataset:

Output of segimage2itkimage (relevant part):

            "SegmentedPropertyTypeCodeSequence" : {
               "CodeMeaning" : "Cerebral hemisphere",
               "CodeValue" : "T-A010F",
               "CodingSchemeDesignator" : "SRT"
            },
            "SegmentedPropertyTypeModifierCodeSequence" : {
               "CodeMeaning" : "Right",
               "CodeValue" : "G-A100",
               "CodingSchemeDesignator" : "SRT"
            },

Presentation in Slicer Segmentations module (modifier not initialized):

image

cpinter commented 7 years ago

Is it maybe an update issue? What happens if you click another property then back?

cpinter commented 7 years ago

I'm trying to build DCMQI so that I can try it (I have a build error, but I'll try some things first).

In the meantime a question: you mention in the title that the terminology was loaded from Json, but I see no Json file. Can you attach that? Or you mean the segment descriptor Json that the converter creates from the SEG files?

fedorov commented 7 years ago

Or you mean the segment descriptor Json that the converter creates from the SEG files?

Yes, I just looked at the JSON file that was produced by the converter.

If you use the current master of Slicer, you will need to make sure you use the latest master of dcmqi, since otherwise there will be an API mismatch with dcmtk, due to the upgrade of dcmtk in Slicer last night.

cpinter commented 7 years ago

Yes, I updated DCMQI but then I realized I need to rebuild DCMTK

cpinter commented 7 years ago

I updated and built eberything, and loaded the dataset. I confirm that there is no type modifier showing up. However I canont find the json file. There is a folder created c:\Users\pinter\AppData\Local\Temp\QIICR\SEG\2017-04-28_114148 and I think it should be there, because that's where the json files were put before, and the date in the folder name indicates it should contain it. But it's all empty. Any ideas?

fedorov commented 7 years ago

I think I experienced the same issue. I don't know if the temp dir is removed by our code, or something else, and didn't want to spend time investigating, so I just took the command line from the log, and re-ran it manually to get the JSON.

Now that I looked into the code, it does appear that the temp dir is cleaned up upon completion: https://github.com/QIICR/QuantitativeReporting/blob/master/QuantitativeReporting.py#L549-L553.

cpinter commented 7 years ago

Thanks! Probably I'll just comment that out for debugging.

cpinter commented 7 years ago

I could work on this a bit. The cleanupTemporaryData function is called when saving a report, not when loading a SEG. @che85 If you have any comments, please don't hold it back. You can easily save half an hour for me by a half-minunte comment :) Thanks

fedorov commented 7 years ago

@cpinter Christian is on vacation until the end of this week. If you like, we can wait until he returns? Next week is fine, there is no extreme urgency.

cpinter commented 7 years ago

I can keep trying for a bit more. I managed to quickly save the json file, but it did not contain modifiers. Can you tell me which series is the one you had the json for? Thanks!

cpinter commented 7 years ago

Found it, nevermind! left_right_gm_wm_snomedhas modifiers.

fedorov commented 7 years ago

right - sorry - only those that have "snomed" in the name contain modifiers!

cpinter commented 7 years ago

One more question (sorry for the flood!): Looking at the logic code, modifiers were supported by looking for Modifier tags under SegmentedPropertyTypeCodeSequence. Maybe the convention has been changed sometime in the past, but the info was not propagated. Or just another faulty assumption on my part... Should we support more types of mata.json structures, or should I just make it handle the newest one?

fedorov commented 7 years ago

No worries re flood :)

I don't think this part changed, but of course I could be wrong. Modifiers are supposed to be at the same level as the category/type. The organization is defined by this section of the schema: https://github.com/QIICR/dcmqi/blob/master/doc/schemas/seg-schema.json#L109-L141. I think fixing modifier is sufficient for now.

cpinter commented 7 years ago

You're right, after another look, the code also suggests it was always like that. So my job will be somewhat harder than it looked a few mintes ago :)

cpinter commented 7 years ago

It was a simple fix at the end: https://github.com/Slicer/Slicer/commit/1c4e327436dc280dad4fddff8a2f47a714fdc2a3

Type modifier is now correctly populated, at least on my end. Please confirm on yours.

fedorov commented 7 years ago

Nice! I will check in the tomorrow's nightly, since I don't have a recent Slicer build. Thank you!

fedorov commented 7 years ago

Confirmed the issue is resolved in today's nightly. Thank you!