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.

Sign up for GitHub

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

Jump to bottom

secondary capture spacing #4574

Merged
merged 2 commits into from
Apr 12, 2024
Merged

secondary capture spacing #4574

merged 2 commits into from
Apr 12, 2024

Conversation

thewtex
Copy link
Member

@thewtex thewtex commented Apr 11, 2024

  • ENH: Add more tests for DICOM Secondary Capture Spacing
  • BUG: Check for Secondary Capture spacing following DICOM Part 3 Sect A.8.1.3

thewtex and others added 2 commits April 11, 2024 14:28
@thewtex @seanm
ENH: Add more tests for DICOM Secondary Capture Spacing
5c93098 
This updates the Lily.mha baseline to including spacing data and adds a
new DICOM Secondary Capture input and check for an output spacing for 0.5, 0.5 instead
of 1.0, 1.0 per the discussion in
InsightSoftwareConsortium#4521.

Co-authored-by: Sean McBride <[email protected]>
@thewtex @issakomi
@malaterre @pieper
BUG: Check for Secondary Capture spacing following DICOM Part 3 Sect …
86a0ad5 
…A.8.1.3

Following the Secondary Capture Image IOD Module Note:

> If Image Position (Patient) (0020,0032) and Image Orientation (Patient) (0020,0037) (from the Image Plane M  odule) are present, then the values of Pixel Spacing (0028,0030) (from the Image Plane Module and the Basic Pixel   Spacing Calibration Macro included from the SC Image Module) are intended to be used for 3D spatial computations  , rather than any values of Nominal Scanned Pixel Spacing (0018,2010) (from the SC Image Module), which may also   be present.

However, we make sure to throw a warning in `ImageHelper::GetSpacingTagFromMediaStorage` when the tag for a S  econdary Capture MediaStorage is requested and SecondaryCaptureImagePlaneModule is enabled because returning a si  ngle tag is insufficient for this requirement. This function should not be called on this type of media storage.

Pushed upstream here: malaterre/GDCM#170

Co-authored-by: Mihail Isakov <[email protected]>
Co-authored-by: Mathieu Malaterre <[email protected]>
Co-authored-by: Steve Pieper <[email protected]>
@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:IO Issues affecting the IO module area:ThirdParty Issues affecting the ThirdParty module labels Apr 11, 2024
pieper
pieper approved these changes Apr 11, 2024
View reviewed changes
Copy link
Contributor

@pieper pieper left a comment

Choose a reason for hiding this comment

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

Looks correct as far as I can see (didn't build and test)

@thewtex thewtex mentioned this pull request Apr 11, 2024
Merged
@thewtex thewtex requested review from phcerdan and hjmjohnson April 11, 2024 19:01
@hjmjohnson hjmjohnson merged commit c23d6c7 into InsightSoftwareConsortium:master Apr 12, 2024
13 checks passed
@thewtex thewtex mentioned this pull request Apr 12, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjmjohnson hjmjohnson hjmjohnson approved these changes

@pieper pieper pieper approved these changes

@malaterre malaterre Awaiting requested review from malaterre

@issakomi issakomi Awaiting requested review from issakomi

@seanm seanm Awaiting requested review from seanm

@phcerdan phcerdan Awaiting requested review from phcerdan

Assignees
No one assigned
Labels
area:IO Issues affecting the IO module area:ThirdParty Issues affecting the ThirdParty module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thewtex @pieper @hjmjohnson