Skip to content

Conversation

@ieee8023
Copy link
Member

@ieee8023 ieee8023 commented Jun 9, 2025

Also ran black formatter on the files.

@ieee8023 ieee8023 requested a review from Copilot June 9, 2025 00:47

This comment was marked as outdated.

@ieee8023 ieee8023 requested a review from Copilot June 9, 2025 00:50

This comment was marked as outdated.

@ieee8023 ieee8023 requested a review from Copilot June 9, 2025 00:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for DICOM files in the image processing pipeline and applies Black-style formatting across utility and script files.

  • Implements a 128-byte preamble check for “DICM” in load_image to route DICOM files through read_xray_dcm
  • Refactors exception messages and multiline call formatting in torchxrayvision/utils.py
  • Updates scripts/process_image.py to use the new load_image function and applies consistent quoting and line breaks

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
torchxrayvision/utils.py Add DICOM preamble detection in load_image, refactor formatting and exception types
scripts/process_image.py Replace manual imread/normalize logic with load_image, clean up argument parsing formatting
Comments suppressed due to low confidence (4)

torchxrayvision/utils.py:211

  • The key used to track normalization warnings is inconsistent ("norm_check" vs "norm_correct"), causing the check to never be bypassed; consider using a single consistent key.
if not "norm_check" in warning_log:

torchxrayvision/utils.py:81

  • The newly added DICOM detection branch in load_image doesn’t appear to have accompanying tests; consider adding unit tests to cover reading .dcm files.
with open(fname, "rb") as f:

scripts/process_image.py:21

  • [nitpick] The "-f" argument is defined but not used anywhere in the script; consider removing it to reduce confusion.
parser.add_argument("-f", type=str, default="", help="")

torchxrayvision/utils.py:57

  • [nitpick] Replace this generic Exception in normalize with a more specific type like ValueError to make error handling clearer.
raise Exception(

@ieee8023 ieee8023 enabled auto-merge (squash) June 9, 2025 00:58
@ieee8023 ieee8023 merged commit 02d92fa into master Jun 9, 2025
3 checks passed
@ieee8023 ieee8023 deleted the dicom-process-script branch June 9, 2025 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant