Multi-frame DICOM redaction with identical-frame deduplication (fixes #1731, #1737)#2166
Open
HenryVarro666 wants to merge 2 commits into
Open
Multi-frame DICOM redaction with identical-frame deduplication (fixes #1731, #1737)#2166HenryVarro666 wants to merge 2 commits into
HenryVarro666 wants to merge 2 commits into
Conversation
DicomImageRedactorEngine previously detected and redacted PII on only the first frame of a multi-frame instance, and raised 'ValueError: Too many dimensions: 3 > 2' on multi-frame (e.g. XA) grayscale data because the pixel array was passed to Image.fromarray with a frame axis still present. Detection now runs per frame (multi-frame DICOMs can carry burned-in PHI on any frame, not necessarily in the same location) and the redaction write-back applies each frame's boxes to that frame. Frame count is read from Number of Frames (0028,0008); single-frame behavior is unchanged. - _get_number_of_frames / _image_from_array / _rescale_array helpers - per-frame rescaling preserves per-frame contrast - _add_redact_box decodes pixel data once and writes it back as one block, fixing the fragile re-decode in the previous in-place mutation - verify engine visualizes a representative frame for multi-frame inputs - unit tests for greyscale/color multi-frame redaction and the data-privacy-stack#1731 crash Fixes data-privacy-stack#1737, data-privacy-stack#1731.
Byte-identical frames (static lead-in/out or padding frames in cine loops) produce identical OCR + analysis results. Cache detection results by a SHA-256 of the frame bytes so each distinct frame is analyzed once; every frame is still redacted. Resolves the per-frame OCR cost concern raised in the data-privacy-stack#2094 review discussion. Single-frame paths skip hashing entirely. Also normalizes formatting of the touched files with the pinned ruff version (0.4.3).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends presidio-image-redactor to properly support multi-frame DICOM instances by performing per-frame OCR/PII detection and applying redaction to each frame, while deduplicating byte-identical frames to avoid repeated OCR cost. It also updates the DICOM verification engine to show a representative frame for multi-frame inputs and documents the feature in the changelog.
Changes:
- Implement per-frame analysis + redaction for multi-frame DICOMs, with SHA-256-based identical-frame deduplication.
- Add a dedicated unit test suite for synthetic single-/multi-frame grayscale and RGB DICOM redaction, including deduplication behavior.
- Update the DICOM verify engine to safely handle multi-frame inputs by visualizing a single frame.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| presidio-image-redactor/presidio_image_redactor/dicom_image_redactor_engine.py | Implements frame-aware analysis/redaction and identical-frame deduplication; refactors pixel-array handling for multi-frame safety. |
| presidio-image-redactor/presidio_image_redactor/dicom_image_pii_verify_engine.py | Ensures verify flow handles multi-frame images by selecting a representative frame for visualization. |
| presidio-image-redactor/tests/test_dicom_multiframe_redaction.py | Adds comprehensive unit coverage for multi-frame behavior and deduplication. |
| CHANGELOG.md | Documents the new multi-frame DICOM redaction support and deduplication behavior. |
Comment on lines
72
to
+76
| is_greyscale = self._check_if_greyscale(instance) | ||
| image_np = self._rescale_dcm_pixel_array(instance, is_greyscale) | ||
| if is_greyscale: | ||
| # model L for grayscale, and has 8 bit-pixel to store the pixel value | ||
| image_pil = Image.fromarray(image_np, mode="L") | ||
| else: | ||
| # model RGB, has 3x8 bit pixel available to store the value | ||
| image_pil = Image.fromarray(image_np, mode="RGB") | ||
| padded_image_pil = self._add_padding(image_pil, is_greyscale, padding_width) | ||
| num_frames = self._get_number_of_frames(instance) | ||
| is_multiframe = num_frames > 1 and image_np.shape[0] == num_frames | ||
|
|
Comment on lines
83
to
+90
| is_greyscale = self._check_if_greyscale(instance_copy) | ||
| image = self._rescale_dcm_pixel_array(instance, is_greyscale) | ||
| num_frames = self._get_number_of_frames(instance) | ||
| if num_frames > 1 and image.shape[0] == num_frames: | ||
| # Verification visualizes a single representative frame; redaction | ||
| # (DicomImageRedactorEngine) handles every frame of a multi-frame | ||
| # instance. | ||
| image = image[0] |
| ### Image Redactor | ||
| #### Added | ||
| - Added Azure SDK credential support to `DocumentIntelligenceOCR` so callers can use Azure Identity credentials instead of API keys (#2085) (Thanks @mturac) | ||
| - Multi-frame DICOM redaction support in `DicomImageRedactorEngine`. PII is now detected and redacted on every frame of a multi-frame instance (`NumberOfFrames > 1`), rather than only the first frame. Byte-identical frames (e.g., static lead-in or padding frames in cine loops) are analyzed once and reuse their detection results, so OCR cost scales with the number of distinct frames. This also resolves the `ValueError: Too many dimensions: 3 > 2` raised when redacting multi-frame (e.g., XA) grayscale instances (#2094). Fixes #1737, #1731. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Description
Multi-frame DICOM redaction for
DicomImageRedactorEngine. Fixes the #1731 crash on multi-frame (e.g. XA) grayscale instances and implements per-frame detection/redaction as requested in #1737, with identical-frame deduplication to keep OCR cost proportional to distinct frames.This builds directly on #2094 by @Dashtid (cherry-picked with authorship preserved), who closed it CI-green and invited pickup. On top of his work, this PR settles the design question he left open:
What was wrong
redact_and_return_bbox()passed the full 3-D pixel array(frames, rows, cols)toImage.fromarray(..., mode="L"), which raisesValueError: Too many dimensions: 3 > 2(newer Pillow:TypeError: Cannot handle this data type). Even without the crash, only the first frame would have been scanned — burned-in PHI can appear on any frame and at different positions per frame.Issue Reference
Fixes #1731, fixes #1737. Supersedes (and includes) #2094; earlier attempt #1735 has the original discussion.
Verification
Unit tests:
tests/test_dicom_multiframe_redaction.py— 17 tests on synthetic single/multi-frame instances (greyscale + RGB), including two new deduplication tests (identical frames analyzed once while still all redacted; distinct frames each analyzed). Fullpresidio-image-redactorsuite: 311 passed, 15 skipped.ruff check/ruff format(pinned 0.4.3) pass.End-to-end on the exact file from #1731 (Siemens Enhanced XA conformance sample, 133 frames of 960×1240, 16-bit MONOCHROME2):
main: crash reproduced atImage.fromarray(the reported error).PATIENT: JOHN DOE DOB 01/01/1980on frames 0–2 (top-left) and 60–62 (bottom-right) yields 20 detections, and exactly frames [0, 1, 2, 60, 61, 62] are modified — per-frame detection catches PHI at different positions on different frames, and the other 127 frames are untouched.