Segmentation Parsing

[CPBridge]

Implementation of substantial parts of the planned segmentation parsing.

This begins with implementation of from_dataset/from_sequence classmethods and "getter" properties for several classes:

• AlgorithmIdentificationSequence
• PixelMeasuresSequence
• PlanePositionSequence
• PlaneOrientationSequence
• SegmentDescription
• Segmentation

In order to check each of the above sequences for the required attributes in a less verbose, manual way, I implemented checks that a dataset contains the required attributes from a certain module automatically using the module definitions contained within highdicom. For now this functionality is private and lives in highdicom._module_utils.

Beyond this, there are several new methods on the Segmentation class that allow users to get various properties of the segmentation image, search for segments and tracking id/uids using filters, and get pixel arrays. There are currently three different public methods for constructing pixel arrays:

1. get_pixels_by_source_instance - This is intended for segmentations derived from (multiple) single-frame instances, where frames are indexed by the SOP Instance UID of the source frame.
2. get_pixels_by_source_frame - This is intended for segmentations derived from multiple-frame intances, where frames are indexed by the the frame number of the source frame.
3. get_pixels_by_dimension_index_values - Allows users to index by arbitrary dimension index values. This allows for a more general case where the segmentation may de derived from resampled source images.

Considered out of scope for this PR but planned for future work:

• Lazy decoding of segmentation frames via subclassing ImageFileReader
• from_dataset method for DimensionIndexSequence - awaiting changes to the encoding behaviour in light of recent discussions around dimension organization uids.
• Indexing segmentations by spatial locations (without interpolation)

Everything seems to be working fine but this PR is currently provisional until tests are written.

@hackermd could you please comment on the general approach and the interface before tests are written?

[hackermd]

Looks nice @CPBridge. I have added a few suggestions and comments here and there, but my two main concerns are:

1. Handling of single-frame and multi-frame source images: The main question is whether we should try to take a general approach that would work for both cases, or implement specific methods for each case. I am ambivalent and see pros and cons for either approach.
2. Types of return value of properties: Several of the properties assume that the values stored on the corresponding attributes have a specific highdicom type (e.g., CodedConcept). The type is enforced by the constructor methods (either via __init__() or from_dataset()/from_sequence()). However, I am wondering whether the properties should ensure that the returned values have the expected type (if it doesn't introduce too much overhead), because it's easy to miss the type casting in the alternative constructor methods and because the objects are mutable. We may also want to revisit the SR pull request.

[CPBridge]

@hackermd I have added full tests and this is ready for a full review. A couple of things to note since you last looked:

• I added some test files for CT segmentations to enable testing different aspects of the decoding
• I also added a README file in the test data directory so we can make notes about what all the test files are. I wasn't sure about some of the files that you added (especially the different SM segmentations), so maybe you could update that file with brief notes.
• I added an option to rescale fractional segmentations back to the 0.0 to 1.0 range as the previous implementation did not have this. It is on by default but can be turned off if desired.

[CPBridge]

I'm not sure why the travis pipeline isn't running, and I can't see how to trigger it

[hackermd]

Very nice @CPBridge! I only have a couple of suggestions for your consideration, but am also fine with merging as is.

[hackermd]

Do we want to rewrite this into a list comprehension for (potentially) improved performance?

[CPBridge]

This can't be written as a list comprehension because the check is applied to the list that is being constructed. It appends the segmented property to the list only if it is not already in the list.

(It is also not possible to use a set here because CodedConcept is not hashable).

I think we are stuck with it the way it is, even if it's a bit ugly.

[hackermd]

This can't be written as a list comprehension because the check is applied to the list that is being constructed. It appends the segmented property to the list only if it is not already in the list.

Right...

It is also not possible to use a set here because CodedConcept is not hashable

This is something we should consider adding. We may also want to add __hash__to pydicom.sr.coding.Code so that we can use instances as dictionary keys..

[CPBridge]

@hackermd a few changes since the last review:

• Added more docstring examples for the more complicated methods
• Removed Optional from the docstrings as requested, in favour of Union[x, None]
• Added a new segread function (analogous to dcmread) to read from a file into a segmentation object in a single line of code. I wanted to put this in hd.seg.utils but couldn't because of circular imports, so put in hd.seg.sop instead. We may want to add srread similarly?

I think all previous comments have now been addressed and I don't have plans at the moment to add any more functionality in this PR.

[hackermd]

Thanks @CPBridge. I would suggest merging this into master, but wait for #69 before drafting a new release.

[CPBridge]

I agree on waiting for #69 to release. Feel free to merge this to master now if you like, or we can try to combine this with #69 in a dev branch first, up to you

[CPBridge]

Actually, since it seems like there are still a few potentially gnarly things to resolve on #69, I think I recommend holding this here until we finalize both PRs. We may make some decisions on #69 with broad implications for this PR too, and we should aim for consistency of approach between the two.

[CPBridge]

Merging into dev branch to continue refinement there alongside #69