Review ClearCanvas plugin terminology for reuse #52
Comments
|
To clarify this issue, anatomic region and segmentation category need to be populated in DICOM SEG, see here as an example: https://github.com/QIICR/Iowa2DICOM/blob/master/ConvertSegmentations/EncodeSEG.cxx#L346-L352 The idea is to have a free text dynamic search based on code meaning for the the region and modifier, and for category and its modifier. It is a quite long list, and I don't know how dynamic search is implemented in Qt or other tools we have. That needs to be investigated. Let me know if we need to discuss more. |
|
The Models module has a "scroll to" search box, and the color picker pop up also has a search box, so there are some examples I can follow. |
|
The module search box in the main toolbar is also a good one. |
|
@fedorov: after our meeting today, my understanding of the phases of this change are:
To keep in mind:
|
|
Yes, this is good. Few clarifications:
|
|
For clarification, in the Reporting implementation there's a structure with: Can you explicity map out the correspondence for me between these values and the Category, Category Modifier? |
It seems to me that SegmentationCategoryTypeModifier.xml alone will be enough to map something like Slicer GenericAnatomyColors (http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/Extensions/Reporting). You have a list of categories, then depending on a category you initialize type selector, and depending on category/type, you initialize the modifier selector. AnatomicRegion + modifier are optional. So the way I see it is to have a separate selector with all anatomic regions, and a selector with modifiers as needed based on the selected region.
Yes, absolutely. At least when the search is done, it should be by the text in the meaning. When a code is selected, it probably makes sense to communicate the triple to the user as a tooltip, or as a static text underneath the selectors. |
|
@fedorov I refactored the color logic[1] to provide an API to create new terminology mappings for color nodes. The commit message includes a python example, calling AddTermToTerminology will create a new mapping if it doesn't exist for the given LUT and then create the structures needed to add the passed strings to the terminology. I'm still considering breaking it down more (like allowing adding just the category by itself, then the type, then the modifier), let me know what you think about this level of detail. |
|
@naucoin here is the branch of Reporting updated to read SEG objects with multiple segments: https://github.com/fedorov/Reporting/tree/refactor-to-new-dcmtk The specific location you need to update to hook up color and terminology are here: https://github.com/fedorov/Reporting/blob/refactor-to-new-dcmtk/Py/DICOMSegmentationPlugin.py#L124-L130 Let's discuss sometime soon. I think it should be relatively straightforward to finish this up and have multi-segment SEGs reading integrated. Currently, each segment is loaded into a separate label node, but I think I might later implement a check that takes all segments that do not overlap and merges them into one label node. Need to think what is the right approach. In a related development, I talked with @pieper today, and he is looking into integration of writing multi-segment SEGs using the same approach used for reading (the groundwork is done by a CLI, and python for integration). For writing, the CLI to be used as the basis is here: https://github.com/QIICR/Iowa2DICOM/blob/master/ConvertSegmentations/EncodeSEG.cxx |
|
Looks straight forward. Do you have a sample .info file? |
|
The text in the comment in the code is literally cut and paste from the info file. I will send you a sample SEG so you can generate one. |
|
Nicole, I sent you a sample SEG with 10 segments by email. With the branch I referenced, you should be able to import it into the DICOM browser, and load all 10 segments as separate labels. |
|
I built your branch and tried to load the tumor_User1_Manual_Trial1.dcm file, working through a few issues in the code (it crashed because I usually start without CLIs loaded, debugging a labelName missing issue). |
|
You cannot start without CLIs loaded, the plugin is dependent on the SEG2NRRD CLI bundled in the extension. |
|
It shouldn't crash though, so I'm putting in a warning. |
When reading in a DICOM segmentatin file, create one color table node for all the segments, and build up a terminology associated with the segmentation from the .info files. Adjusted the logic call to not use the color node, do the terminology and color information file parsing in the segmentation plugin. Added a check that the SEG2NRRD CLI was loaded to prevent a crash if Slicer was started up without CLI modules. Issue QIICR#52
When reading in a DICOM segmentatin file, create one color table node for all the segments, and build up a terminology associated with the segmentation from the .info files. Adjusted the logic call to not use the color node, do the terminology and color information file parsing in the segmentation plugin. Added a check that the SEG2NRRD CLI was loaded to prevent a crash if Slicer was started up without CLI modules. Issue QIICR#52
|
FYI here's how "SEG browsing" interface looks like in ClearCanvasAIM
|
|
As a side note, I have to admit I like a lot how units are natively connected with the measurements in ClearCanvas:
|
|
SEG2NRRD uses the voxel value 1 for all segmentations, so the color mapping isn't working correctly. Any objections to me changing it to use the segmentId or does it need to be filled with just 0 and 1 values? |
|
Setting the color table color names to the full terminology is not working well with the data probe, the screen capture shows how it was expanded after I moused over the segment. |
|
Updated editor with Region |
|
OK to assign segment number in place of 1. Yes, the names are too long. I think the best alternative is to have a specialized term browser. But I think this is not immediate priority. For now, maybe you can look into setting full term names as tooltips for the colors table rows? |
|
I know I could do a right click on the color table rows to pop up terms, I'll have to look into tool tips. Another option would be to have the same set up as in the editor where when you click on a color in the colors module, it updates the terminology widgets. I'd probably translate the editor scripted panel into a c++ widget that can be more easily reused in that case though. |
After updates to the Slicer topic branch in naucoin 4047-Add-semantic-color-information-to-color-selector-in-editor parse out and set the region information read from the segmentation object. Take into account that the modifier lines may be omitted in the .info file, refer to SEG2NRRD for the calls that do the writing. Updated SEG2NRRD to set the label map pixel values to the segmentation number rather than always using 1. This allows the colors to display properly with one color node rather than having to define a color node per segmentation. Issue QIICR#52
|
Table items tooltips: http://doc.qt.io/qt-4.8/qtablewidgetitem.html#setToolTip |
|
I would also prefer single column for displaying the items, and as discussed, full terminology concatenation should not show up in the DataProbe. |
When parsing the .info files that come with segmentation label images, some terminology information may be missing. This change sets some default values. Issue QIICR#52
|
Tool tips in the Colors module: |
Update the plugin to use the new order of terms in adding a term to a terminology. Update the SEG exporter self test to use the new color logic methods, TBD: constructing the proper format text files. Depends on: naucoin/Slicer@5086c5e Issue QIICR#52
Update the plugin to use the new order of terms in adding a term to a terminology. Update the SEG exporter self test to use the new color logic methods, TBD: constructing the proper format text files. Depends on Nov 11/15 update to: https://github.com/naucoin/Slicer/tree/4047-Add-semantic-color-information-to-color-selector-in-editor Issue QIICR#52
|
@fedorov @pieper Reporting update to use the new methods and trying to update the self test, but @pieper I wasn't able to quite figure out which terms you needed, I put all of them there so you can use them (nb: the code meaning, value, scheme may not be in the right order from the concatenated util methods that I added). |
|
The changes look great! Some smallish things... In the comment you have CodeValue,CodeMeaning,CodingSchemeDesignator, but Here's where they are parsed: https://github.com/naucoin/Reporting/blob/master/SEGSupport/SegmentAttributes.h#L233-L238 So they should not be in the order listed in the comment (either the Also as you have the self test now it implies that propertyType can be https://github.com/naucoin/Reporting/blob/master/SEGSupport/EncodeSEG.cxx#L339-L345 Also, if something like SegmentedProperytCategory aren't defined in the A more big picture question which you guys may have thought through On Wed, Nov 11, 2015 at 6:42 PM, Nicole Aucoin notifications@github.com
|
|
@pieper In SEGExporterSelfTest code, I hadn't figured out exactly what you were trying to output there since there were some debugging style statements but no comment with the intent (late in the day coding, I saw that the extension was .txt rather than .info and didn't want to assume you were writring the same format as the .info files I get from the segmentation plugin[1]). If you want an error condition added for the non optional ones I can do that, and I'll take another look at the code to try and match the format of the plugin. I double checked that string.replace() on an empty string wasn't causing an exception in the python console, but I can add empty string checks first. [1] https://github.com/naucoin/Reporting/blob/master/Py/DICOMSegmentationPlugin.py#L147 |
Let's bring the power of github here - including @dclunie!! :) As a background for David - Slicer color tables were originally used to establish linkage between the segmentation pixel values, RGB color and essentially free-text label. GeneralAnatomy is one such lookup table that as you may remember we annotated with your help using SNOMED terminology (wiki.slicer.org/slicerWiki/index.php/Documentation/Nightly/Extensions/Reporting). Now we are working to integrate terminology more tightly into Slicer with the SEG export being the main driving use case. The way I thought about the Slicer SEG export and terminology related issues is that we have codes for GeneralAnatomy, and for the color tables that were created dynamically while importing SEGs. For other color tables, we would put a very generic term like "Tissue" for every label. I think "Tissue" would be semantically correct for most cases, although imprecise. Of course, it will not cover cases like foreign objects, table, phantom segmentations etc. I searched just now, and there is code (111176,DCM,Unspecified) in CID 6040 "Non-lesion object types". @dclunie - would it pass your threshold if we use this code for segmentation object segments where the user did not specify the term, or we cannot determine it from predefined mappings? I personally think this would be way better than what some other workstations do. For example, segmentations done in syngo.via seem to populate anatomical information from the BodyType field of the source image, so whatever I segment in a chest CT will be assigned Chest code. |
|
Looks like the DICOMSegmentationPlugin that I was starting from uses a slightly different format (from SEG2NRRD) than EncodeSEG. DICOMSegmentationPlugin: file named label #.info, formatted with line breaks, one colon, comma separted: EncodeSEG is expecting a list of files with a line per label, semi colon separated instead of line breaks (current broken example that I'm trying to fix): Do we want to keep supporting both? |
We can switch SEG2NRRD to use the format used by EncodeSEG. If you look at EncodeSEG, it has code to parse those files that perhaps can be reused in Slicer. The reason EncodeSEG has that format is because inputs can have multiple labels. I was thinking to augment SEG2NRRD later with a capability to merge segments, so it will be handy as welll. We can do this later -- the format used by EncodeSEG is not parsed by Slicer, so I think not much support is needed beyond writing out semicolon separated strings. |
Bring code to conform to this change in the order of arguments, driven by the standard order expected of the standard term elements: naucoin/Slicer@ab484bf Fix the self test to use the correctly ordered return triple, it passes in my test. Issue QIICR#52
|
This version of the .txt file from EncodeSEG lets the self test pass: |
Update SEG2NRRD.cxx to use the format from EncodeSEG.cxx. Update DICOMSegmentationPlugin.py to parse the single line format, with the capability to deal with multiple lines defining terminology for different labels in one file. STYLE: updated variable names in the plugin to use CodeMeaning instead of Name and CodingScheme instead of Scheme. Issue QIICR#52
Update the plugin to use the new order of terms in adding a term to a terminology. Update the SEG exporter self test to use the new color logic methods, TBD: constructing the proper format text files. Depends on Nov 11/15 update to: https://github.com/naucoin/Slicer/tree/4047-Add-semantic-color-information-to-color-selector-in-editor Issue QIICR#52
Bring code to conform to this change in the order of arguments, driven by the standard order expected of the standard term elements: naucoin/Slicer@ab484bf Fix the self test to use the correctly ordered return triple, it passes in my test. Issue QIICR#52
Update SEG2NRRD.cxx to use the format from EncodeSEG.cxx. Update DICOMSegmentationPlugin.py to parse the single line format, with the capability to deal with multiple lines defining terminology for different labels in one file. STYLE: updated variable names in the plugin to use CodeMeaning instead of Name and CodingScheme instead of Scheme. Issue QIICR#52
Update the plugin to use the new order of terms when adding a term to a terminology. Update the SEG exporter self test to use the new color logic methods, Depends on Nov 12/15 update to: https://github.com/naucoin/Slicer/tree/4047-Add-semantic-color-information-to-color-selector-in-editor Unify terminology file formats: Update SEG2NRRD.cxx to use the format from EncodeSEG.cxx. Update DICOMSegmentationPlugin.py to parse the single line format, with the capability to deal with multiple lines defining terminology for different labels in one file. Issue QIICR#52
|
Slicer pull request: |
|
@pieper things seem to work fine, so the only issue remaining is the support of export at the user level from the subject hierarchy. |
|
Great - now we're down to just the interface issues. Putting it in the On Fri, Nov 13, 2015 at 3:59 PM, Andrey Fedorov notifications@github.com
|
|
Infrastructure changes commited to Slicer: |
|
This issue is addressed separately by handling terminology directly in the new Segmentation Editor. This is under development by @cpinter |
See
Segmentation/Configuration/AnatomicRegionAndModifier.xml
Segmentation/Configuration/SegmentationCategoryTypeModifier.xml
in https://github.com/fedorov/ClearCanvas-AimPlugin4.5
The text was updated successfully, but these errors were encountered: