-
Notifications
You must be signed in to change notification settings - Fork 521
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
Improve segmentation (.seg.nrrd) file reader robustness #7738
Improve segmentation (.seg.nrrd) file reader robustness #7738
Conversation
"Segment0_ID:=" string may not be found in the first 800 characters if segment metadata is placed after common segmentation metadata fields. The code now looks for a common segmentation field prefix ("Segmentation_"), to handle this case as well.
1. Fixed extent computation when reading from a segmentation that had shared labelmaps. Extent of the first segment in that layer was used. Instead, union of extents of all the segments in that layer should be used. 2. Fixed parsing error of Segmentation_ContainedRepresentationNames and SegmentN_Tags: previously, a "|" was required at the end of the list (e.g., "Binary labelmap|Closed surface|"). Now the closing separator is optional.
{ | ||
currentSegmentExtent[i] = imageExtentInFile[i]; | ||
GetImageExtentFromString(currentSegmentExtent, currentExtentString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the extent string is invalid, then the extent of the current segment will be skipped.
Should we instead switch to the imageExtentInFile if we encounter an invalid extent string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By invalid do you mean empty (something like [0, -1, 0, -1, 0, -1])? That would mean that the segment is empty and so it should not expand the extent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I meant that if the string itself was invalid, and could not be correctly parsed.
Edit:
If the extent string is invalid, then the extent of the current segment will be skipped.
In retrospect, this may or may not be true depending on how the string is invalid. If nothing can be parsed then [0, -1, 0, -1, 0, -1] will be returned. If the first few values can be read, then the extent may be partially defined by just the first few values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed all these parsing codes are still not fully robust for invalid input. The changes in this commit improve things, so I would integrate it as is, and implement further improvements later if needed.
Make the segmentation storage node read .seg.nrrd files that are created outside of Slicer (e.g., using slicerio Python package).
They may use different order of fields (therefore the file detection heuristics had to be tuned), some fields may be missing, and there could be minor syntax differences (e.g., Slicer always terminates a list with separator character, but other software may just use separators between components).