Add a DTO for usage with the Django app. #21

zgypa · 2025-10-11T03:58:22Z

Adds a Django idiomatic model to the bfd9000_dicom package. Major refactoring and re-organization.

…DICOM metadata models for various imaging modalities

…ance UIDs for improved type safety

…tiff2dcm.py

Copilot

Pull Request Overview

This PR adds a Django idiomatic DTO (Data Transfer Object) architecture to the bfd9000_dicom package, with major refactoring and reorganization of the codebase. The changes introduce a clean separation of concerns with specialized converter classes, core utilities, and Django-style metadata models for various imaging modalities.

Key changes:

Introduced Django-style DTO models for DICOM metadata with multiple imaging modalities support
Restructured the package into logical modules: converters/, core/, and models.py
Added comprehensive test coverage and documentation for the new architecture

Reviewed Changes

Copilot reviewed 28 out of 35 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
bfd9000_dicom/bfd9000_dicom/models.py	Implements Django-style DTO classes for DICOM metadata across multiple modalities
bfd9000_dicom/bfd9000_dicom/converters/	New package containing specialized converters for different imaging types
bfd9000_dicom/bfd9000_dicom/core/	Core utilities for DICOM building and compression, refactored from existing modules
bfd9000_dicom/tests/	Comprehensive test suite for the new architecture and existing functionality
bfd9000_dicom/examples/basic_usage.py	Example code demonstrating the new DTO and converter APIs
bfd9000_dicom/docs/	Architecture documentation and planning documents
bfd9000_dicom/pyproject.toml	Project configuration with updated dependencies and metadata

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-11T04:00:47Z

bfd9000_dicom/bfd9000_dicom/tiff2dcm.py

 import os
 import json
-import pydicom
 import argparse
 from pydicom.dataset import Dataset
 from pydicom.uid import SecondaryCaptureImageStorage, generate_uid


The removal of the import pydicom line may indicate incomplete refactoring. Verify that all pydicom functionality is accessed through specific imports rather than the general module import.

Copilot · 2025-10-11T04:00:47Z

bfd9000_dicom/plan.md

+                f"{self.nominal_scanned_pixel_spacing[0]}",
+                f"{self.nominal_scanned_pixel_spacing[1]}"
+            ]
+            ds.PixelSpacing


Incomplete code line - the assignment to ds.PixelSpacing is missing its value. This appears to be truncated code that should be completed or removed.

Suggested change

ds.PixelSpacing

ds.PixelSpacing = [

f"{self.nominal_scanned_pixel_spacing[0]}",

f"{self.nominal_scanned_pixel_spacing[1]}"

]

Copilot · 2025-10-11T04:00:48Z

bfd9000_dicom/plan.md

+                f"{self.nominal_scanned_pixel_spacing[0]}",
+                f"{self.nominal_scanned_pixel_spacing[1]}"
+            ]
+            ds.PixelSpacing


Duplicate incomplete code line - the assignment to ds.PixelSpacing is missing its value. This appears to be duplicated truncated code that should be completed or removed.

Suggested change

ds.PixelSpacing

ds.PixelSpacing = [

f"{self.nominal_scanned_pixel_spacing[0]}",

f"{self.nominal_scanned_pixel_spacing[1]}"

]

…update usage examples

- Remove obsolete DocumentConverter and Converter classes. - Introduce new JPEGConverter, PNGConverter, PDFConverter, STLConverter, and TIFFConverter classes for handling respective formats. - Implement a router system to automatically select the appropriate converter based on file type. - Add example usage scripts demonstrating the new converter architecture and functionality. - Ensure proper handling of metadata and pixel data for each converter.

Relocated compression.py → compression.py Updated all import statements across the codebase (TIFF, JPEG, PNG converters, tests, etc.) Integrated DICOM tags logic from dicom_tags.py Added Bolton Brush specific attributes to RadiographMetadata class: is_bolton_brush_study flag Automatic setting of Bolton Brush device information when enabled Added orientation methods: set_orientation_ll(), set_orientation_pa(), set_orientation_hand() Integrated common Bolton Brush tags into the metadata DTO Integrated TIFF conversion logic from tiff2dcm.py Created utility functions in converters/utils.py: extract_bolton_brush_data_from_filename() - parses Bolton Brush filename format load_radiograph_metadata_from_json() - backward compatibility for JSON metadata Exported these utilities in the main package API Updated all imports and references Updated main package __init__.py to export new utility functions All compression imports now point to the new location No remaining active references to old modules 📁 File Changes: Moved: compression.py → compression.py Modified: models.py - Added Bolton Brush support to RadiographMetadata Created: converters/utils.py - Bolton Brush utilities Updated: All converter files, main __init__.py, test files 🗂️ Remaining Action: The old/ folder can now be safely removed since all its logic has been integrated into the new architecture:

- Moved exception imports to the base converter module. - Updated import paths in JPEG converter. - Removed unused compression imports from core module. - Improved README test structure and added categories. - Added unit tests for Bolton Brush utilities and DICOM metadata models. - Created integration tests for converter router functionality. - Enhanced TIFF converter tests and added edge case handling.

…ion and update filename parsing to include image type

…e readability

…on calls and clean up whitespace

…action, and add new modality types

Copilot

Pull Request Overview

Copilot reviewed 38 out of 43 changed files in this pull request and generated 5 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-11T22:17:04Z

bfd9000_dicom/bfd9000_dicom/core/dicom_builder.py

    # Calculate PixelAspectRatio using original dpi values to avoid rounding errors
    if dpi_horizontal == dpi_vertical:
-        pixel_aspect_ratio = [1, 1]
+        pixel_aspect_ratio = ["1", "1"]


Inconsistent return type for pixel_aspect_ratio. The function returns a list [\"1\", \"1\"] for equal DPI but a list [f\"{int(dpi_vertical / gcd_ratio)}\", f\"{int(dpi_horizontal / gcd_ratio)}\"] for unequal DPI. The original function returned tuples but this has been changed to lists for string formatting, which should be consistent throughout.

Copilot · 2025-10-11T22:17:04Z

bfd9000_dicom/bfd9000_dicom/models.py

+class ConversionType(Enum):
+    """DICOM Conversion Type values (0008,0064)."""
+    DF = "DF"  # Digitized Film
+    SI = "SI"  # Digital Interface


Corrected 'DI' to 'SI' in the enum value comment. The enum value 'SI' stands for 'Synthetic Image' but the comment incorrectly references 'Digital Interface'.

Suggested change

SI = "SI" # Digital Interface

SI = "SI" # Synthetic Image

Copilot · 2025-10-11T22:17:04Z

bfd9000_dicom/examples/basic_usage.py

+    # Create metadata
+    metadata = RadiographMetadata(
+        patient_id=patient_id,
+        patient_sex=PatientSex[sex],
+        patient_age=age_months, # This might not be the correct format for the DTO.


The comment indicates uncertainty about the correct format for patient_age. Based on the DTO definition, patient_age should be a string in DICOM format (e.g., '217M' for months), not the raw age_months value returned from extract_metadata_from_filename.

Suggested change

# Create metadata

metadata = RadiographMetadata(

patient_id=patient_id,

patient_sex=PatientSex[sex],

patient_age=age_months, # This might not be the correct format for the DTO.

def months_to_dicom_age(months):

"""Convert integer months to DICOM age string (e.g., '217M')."""

return f"{int(months):03d}M"

# Create metadata

metadata = RadiographMetadata(

patient_id=patient_id,

patient_sex=PatientSex[sex],

patient_age=months_to_dicom_age(age_months),

Copilot · 2025-10-11T22:17:05Z

bfd9000_dicom/bfd9000_dicom/converters/jpeg.py

+from pydicom.uid import ExplicitVRLittleEndian, JPEG2000Lossless
+
+from bfd9000_dicom.models import BaseDICOMMetadata
+from .compression import get_encapsulated_jpeg2k_pixel_data


Incorrect relative import path. Should import from bfd9000_dicom.converters.compression instead of .compression since the compression module is in the same converters package.

Suggested change

from .compression import get_encapsulated_jpeg2k_pixel_data

from bfd9000_dicom.converters.compression import get_encapsulated_jpeg2k_pixel_data

Copilot · 2025-10-11T22:17:05Z

bfd9000_dicom/bfd9000_dicom/models.py

+class ConversionType(Enum):
+    """DICOM Conversion Type values (0008,0064)."""
+    DF = "DF"  # Digitized Film
+    SI = "SI"  # Digital Interface


Incorrect comment for enum value. 'SI' in DICOM Conversion Type should be 'Synthetic Image', not 'Digital Interface'. The correct value for Digital Interface would be 'DI'.

Suggested change

SI = "SI" # Digital Interface

SI = "SI" # Synthetic Image

…t in metadata, and improve utility function usage

…h new extractor, update examples and tests accordingly

…olton Brush limits

zgypa added 10 commits October 9, 2025 20:16

Rename bbc2dcm to bfd9000_dicom

75423fc

Update VSCode settings to reflect the new bfd9000_dicom directory path

6c332b7

Add pyproject.toml for project configuration and dependencies

7b043cf

Refactor README.md and add DTO architecture documentation; implement …

92f064a

…DICOM metadata models for various imaging modalities

Refactor DICOM metadata models to use UID type for SOP Class and Inst…

b497352

…ance UIDs for improved type safety

Initial copilot work. No logic yet.

fee012d

Add initial plan for DICOM metadata structure and models

cb7782e

Fix test path in VSCode settings and remove unused pydicom import in …

bba799a

…tiff2dcm.py

Add BFD9000 logo and update README for legacy collection conversion

532328e

Update BFD9000 logo image in documentation with the upside down version.

b4e120a

zgypa self-assigned this Oct 11, 2025

zgypa requested a review from Copilot October 11, 2025 04:00

Copilot AI reviewed Oct 11, 2025

View reviewed changes

zgypa added 12 commits October 11, 2025 00:51

Remove JPEG 2000 handling code and associated plan documentation

7caf174

Update logo image syntax in README for consistency

f006132

Clarify description of converters in README for improved understanding

04ae94b

Refactor radiograph converter: move metadata extraction to utils and …

9538b32

…update usage examples

Refactor Bolton Brush utilities: remove unused metadata loading funct…

f9bb457

…ion and update filename parsing to include image type

Refactor TIFFConverter: remove unnecessary whitespace and improve cod…

4f7c11e

…e readability

Add .pylintrc configuration to disable logging f-string interpolation

96bff09

Refactor converter_examples.py: remove unused convert_to_dicom functi…

30f618a

…on calls and clean up whitespace

Refactor utils and models: clean up whitespace, enhance metadata extr…

65b0e17

…action, and add new modality types

zgypa requested a review from Copilot October 11, 2025 22:16

Copilot AI reviewed Oct 11, 2025

View reviewed changes

zgypa added 3 commits October 11, 2025 19:13

Refactor converters and models: clean up whitespace, update age forma…

161d299

…t in metadata, and improve utility function usage

Refactor metadata extraction: replace deprecated utility function wit…

553f88b

…h new extractor, update examples and tests accordingly

Remove deprecated utils module and update maximum patient number in B…

9f3fbe8

…olton Brush limits

zgypa added 2 commits October 11, 2025 19:53

Fix patient ID in maximum age test case for Bolton Brush data extraction

27d638b

Merge branch 'develop' into feature/20-bfd9000_dicom-dto

a01dc0d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a DTO for usage with the Django app. #21

Add a DTO for usage with the Django app. #21

Uh oh!

zgypa commented Oct 11, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	from .compression import get_encapsulated_jpeg2k_pixel_data
	from bfd9000_dicom.converters.compression import get_encapsulated_jpeg2k_pixel_data

Add a DTO for usage with the Django app. #21

Are you sure you want to change the base?

Add a DTO for usage with the Django app. #21

Uh oh!

Conversation

zgypa commented Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zgypa commented Oct 11, 2025 •

edited

Loading