Skip to content

Restructure and Refactor Header Handling #96

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

Conversation

ebald19
Copy link
Contributor

@ebald19 ebald19 commented Jun 18, 2025

This PR implements the functionality of the proposal in this issue.

Changes

  • restructuring of header handling into subclasses
  • use the aimIO approach to managing header data
  • Read headers into pre-defined data structures instead of byte-by-byte
  • Separate the reading of headers by each sub-header chunk (e.g pre-header, struct header, processing log)
  • fix ISQ writing to rescale back to scanco units
  • fix ISQ writing to keep all original metadata
  • add a test to ensure the writing of an ISQ, without any changes applied, results in the same image

API Differences

  • There are no external changes to how scancoIO is used within the ImageFileReader from ITK.
  • ReadISQHeader and ReadAIMHeader have been removed from ScancoIO and subclassed within the AIMHeaderIO and ISQHeaderIO functions.
  • The new classes created here are helper classes and are not published to ITK with ScancoIO
  • The new class APIs allow reading and writing of specific header sections at a time

Future Changes

Incoming PRs will handle writing to AIM files, testing for AIM v030, and float type compatibility for writing

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

After a short review, this mostly looks good. Some remarks inline. The commit message might be rewritten, to not be a squash log, but rather summarize the changes done.

@ebald19 ebald19 force-pushed the enhancement/incorporate_aimio_1 branch from afb9f73 to eff0146 Compare June 19, 2025 15:47
STYLE: documentation and style fixes

BUG: fix pixel and physical dimension population for writing
- have metadata dictionary populate and save dimension values

BUG: fix microsoft-specific errors and register scancoio for use in test for aim writing

WIP: Have write convert back to Scanco units

Merge changes from incorporate aimio:

    WIP: Move header reading/writing to completely depend on subclass ScancoHeaderIO

    WIP: Have ISQ write all metadata as read in

    WIP: Use AimIO method for reading ISQ

    This method defines header structures and reads header bytes directly into them for access

    - add test for reader update in conversion test

    WIP: Complete reading extended header from data struct

    - add documentation and style fixes

    WIP: have aim reader use struct header datatype

    WIP: switch reading to scancoheaderio class

    WIP: add class files for header reader and writers

    WIP: add class files and definitions for separating header reading and
    writing

Merge changes from main:

    BUG: fix creation date output in header write

    STYLE: Move unrelated data manipulation functions out of main class

    STYLE: Add and update doxygen comments for functions

    STYLE: fix spacing for ITK style checks
    BUG: fix CI and compiler warnings and errors
    BUG: remove depreciated sprintf
@ebald19 ebald19 force-pushed the enhancement/incorporate_aimio_1 branch from eff0146 to 1308359 Compare June 19, 2025 15:51
@dzenanz dzenanz requested a review from mkuczyns June 19, 2025 15:52
@dzenanz
Copy link
Member

dzenanz commented Jun 19, 2025

You seem to have forgotten to apply clang-format, otherwise LGTM.

@dzenanz
Copy link
Member

dzenanz commented Jun 19, 2025

Ah, you realized that too and made quick force-push. Let's wait and see what the CI says.

Copy link
Collaborator

@mkuczyns mkuczyns left a comment

Choose a reason for hiding this comment

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

After a quick review, this looks good to me!

@thewtex thewtex merged commit e815be6 into InsightSoftwareConsortium:main Jun 19, 2025
26 checks passed
@ebald19 ebald19 deleted the enhancement/incorporate_aimio_1 branch June 25, 2025 16:14
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.

4 participants