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

Improve segmentation (.seg.nrrd) file reader robustness #7738

Merged
merged 2 commits into from
May 20, 2024

Conversation

lassoan
Copy link
Contributor

@lassoan lassoan commented May 10, 2024

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).

"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.
@lassoan lassoan added this to the Slicer 5.7 milestone May 10, 2024
@lassoan lassoan self-assigned this May 10, 2024
{
currentSegmentExtent[i] = imageExtentInFile[i];
GetImageExtentFromString(currentSegmentExtent, currentExtentString);
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

@lassoan lassoan merged commit a89a928 into Slicer:main May 20, 2024
6 checks passed
@lassoan lassoan deleted the improve-seg-nrrd-reader-robustness branch May 20, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants