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

Allow annotations to reference multiple images #193

Merged
merged 5 commits into from
Aug 5, 2022

Conversation

hackermd
Copy link
Collaborator

@hackermd hackermd commented Aug 2, 2022

An annotations object may contain multiple measurements, which may have been derived from different images. Therefore, it is important to allow reference of multiple (multi-frame) image instances.

@hackermd hackermd added the enhancement New feature or request label Aug 2, 2022
@hackermd hackermd requested a review from CPBridge August 2, 2022 18:59
@hackermd hackermd self-assigned this Aug 2, 2022
Copy link
Collaborator

@CPBridge CPBridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not read the relevant parts of the standard in detail, but this seems potentially too lax and allow the creation of objects whose meaning is ambiguous. In the case where multiple images are passed, in which image's coordinate system are the annotation graphic data defined? Should there not be a check that the different images have the same frame of reference UID (for SCOORD3D) and/or number of rows and columns (for SCOORD)?

@hackermd
Copy link
Collaborator Author

hackermd commented Aug 4, 2022

Should there not be a check that the different images have the same frame of reference UID (for SCOORD3D) and/or number of rows and columns (for SCOORD)?

That's a good idea.

I realized that we will need to differentiate between the Referenced Image Sequence (Microscopy Bulk Simple Annotations Module) and Referenced Instance Sequence (Common Instance Reference Module). At the moment, we are including a reference for each of the source_images in both sequences.

In my opinion, the design of the Microscopy Bulk Simple Annotations Module is flawed for the use of 2D annotation coordinate type (SCOORD), where coordinates are defined relative to the total pixel matrix of an individual SOP instance, because the assumption that a group of annotations applies to a single image instance may not hold - in particular if measurements are included. For example, objects may be detected in one image and the coordinates will be recorded relative to that image, but then measurements may subsequently be taken from a different image, which will have a different SOP Instance UID but the same Frame of Reference UID. Therefore, I only use 3D annotation coordinate type, where coordinates are defined relative to the frame of reference.

In conclusion, I think we should apply different checks for different annotation coordinate type values. I will update the PR accordingly.

@hackermd hackermd requested a review from CPBridge August 4, 2022 18:19
Copy link
Collaborator

@CPBridge CPBridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one tiny suggestion

src/highdicom/ann/sop.py Outdated Show resolved Hide resolved
Co-authored-by: Chris Bridge <chrisbridge44@googlemail.com>
@hackermd hackermd merged commit 548321b into master Aug 5, 2022
@hackermd hackermd deleted the bug/annotation-reference-source-images branch August 5, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants