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

Bug with the creation of a "SegmentSequence" with multiple elements #14

Closed
roger-schaer opened this issue May 18, 2020 · 7 comments · Fixed by #17
Closed

Bug with the creation of a "SegmentSequence" with multiple elements #14

roger-schaer opened this issue May 18, 2020 · 7 comments · Fixed by #17
Labels
bug Something isn't working

Comments

@roger-schaer
Copy link

Sorry to bother you once again, but I noticed another behavior that may need to be fixed in my opinion.

In "template.py, on line 166, this is specified :

_create_segment_dataset(x[0]) for x in metainfo['segmentAttributes']

But I think it should actually be:

_create_segment_dataset(x) for x in metainfo['segmentAttributes'][0]

I say this because according to the schema from dcmqi, the format should look like this:

"segmentAttributes": [
    [
      {
        "labelID": 5,
        ...
      },
      {
        "labelID": 6,
        ...
      },
      {
        "labelID": 3,
        ...
      }
    ]
  ],

"segmentAttributes" is an array containing another array, which in turn contains the objects with the segment id, description, etc.

With the fix mentioned above, a dictionary that validates with the schema is processed correctly, but without it it only takes the first element of the list of segments. And if I change the structure of the data, the validation fails.

I hope that the explanation is clear enough, thanks again for your help!

@razorx89
Copy link
Owner

Thank you again for reporting this issue! No need to be sorry, I am of course happy that you find my library useful :)

Indeed, this looks wrong. I will fix this and add a test case.

@razorx89 razorx89 added the bug Something isn't working label May 18, 2020
@razorx89
Copy link
Owner

Okay, I had a look at this in more detail. I think the implementation is actually correct, although the schema might not be explicit enough. Have a look at:

https://github.com/QIICR/dcmqi/blob/master/doc/examples/seg-example_multiple_segments.json

Translated to your example, it would look like this:

"segmentAttributes": [
    [
      {
        "labelID": 5,
        ...
      }
    ],
    [
      {
        "labelID": 6,
        ...
      }
    ],
    [
      {
        "labelID": 3,
        ...
      }
    ]
],

The inner array only holds a single object. I don't know why they decided to encapsulate the object into another array.

@razorx89 razorx89 removed the bug Something isn't working label May 18, 2020
@roger-schaer
Copy link
Author

The inner array only holds a single object. I don't know why they decided to encapsulate the object into another array.

Ah yes I see, strange, I had a feeling that the validation failed when I structured it like this, I will try again in any case and let you know, thanks!

@roger-schaer
Copy link
Author

Thanks again for the example, I can now confirm that this works correctly, it is indeed a bit strange that each object is wrapped by an array.

@fedorov
Copy link

fedorov commented May 19, 2020

I understand how this can be confusing. The reason there are arrays within arrays is to support overlapping segments.

The outer array corresponds to the list of input segmentations in ITK-readable format passed to the converter. If your input segmentations are all non-overlapping, this could be a single NRRD or another research format, and each segment within that file can be addressed by the label value.

But if your segmentations are overlapping, or if for whatever reason your input segments are stored in separate files, you need to use the outer list with each entry corresponding to a segmentation file, and the inner list for the labels within individual segmentation file.

If you have a single input file, you do NOT need to wrap individual label entries into inner lists! In this case you would have a single outer list containing a single inner list.

Here's an example generated using dcmqi seg webapp that demonstrates the organization of the metadata JSON for an input segmentation that contains two segments:

{
  "ContentCreatorName": "Reader1",
  "ClinicalTrialSeriesID": "Session1",
  "ClinicalTrialTimePointID": "1",
  "SeriesDescription": "Segmentation",
  "SeriesNumber": "300",
  "InstanceNumber": "1",
  "BodyPartExamined": "BRAIN",
  "segmentAttributes": [
    [
      {
        "labelID": 1,
        "SegmentDescription": "Brain",
        "SegmentAlgorithmType": "MANUAL",
        "SegmentedPropertyCategoryCodeSequence": {
          "CodeValue": "123037004",
          "CodingSchemeDesignator": "SCT",
          "CodeMeaning": "Anatomical Structure"
        },
        "SegmentedPropertyTypeCodeSequence": {
          "CodeValue": "12738006",
          "CodingSchemeDesignator": "SCT",
          "CodeMeaning": "Brain"
        },
        "recommendedDisplayRGBValue": [
          250,
          250,
          225
        ]
      },
      {
        "labelID": 2,
        "SegmentDescription": "Tumor",
        "SegmentAlgorithmType": "MANUAL",
        "SegmentedPropertyCategoryCodeSequence": {
          "CodeValue": "49755003",
          "CodingSchemeDesignator": "SCT",
          "CodeMeaning": "Morphologically Altered Structure"
        },
        "SegmentedPropertyTypeCodeSequence": {
          "CodeValue": "108369006",
          "CodingSchemeDesignator": "SCT",
          "CodeMeaning": "Neoplasm"
        }
      }
    ]
  ],
  "ContentLabel": "SEGMENTATION",
  "ContentDescription": "Image segmentation",
  "ClinicalTrialCoordinatingCenterName": "dcmqi"
}

Hope this makes sense, but let me know if you have any questions.

@razorx89
Copy link
Owner

@fedorov Thank you very much for clarifying on this! Now it makes sense :)

@roger-schaer I will prepare a fix for supporting the correct structure. Since currently only the multi-class (non-overlapping) case is supported for writing, I will assume a single inner array and otherwise raise an exception.

@razorx89 razorx89 reopened this May 19, 2020
@razorx89 razorx89 added the bug Something isn't working label May 19, 2020
@fedorov
Copy link

fedorov commented May 19, 2020

Glad I could clarify this!

And also, dcmqi converter should fail when there is a single input file but multiple inner lists. I think I noticed this earlier, but didn't get to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants