Skip to content

ENH: Ingest ITKIOMeshMZ3 into Modules/IO#6214

Merged
hjmjohnson merged 22 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-iomeshmz3
May 5, 2026
Merged

ENH: Ingest ITKIOMeshMZ3 into Modules/IO#6214
hjmjohnson merged 22 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-iomeshmz3

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Ingests ITKIOMeshMZ3 (read/write of Surf Ice's MZ3 surface-mesh format via itk::MZ3MeshIO + factory) into Modules/IO/IOMeshMZ3/ using the v4 ingestion pipeline. Tier-A Wave-1 candidate from #6160. Upstream: InsightSoftwareConsortium/ITKIOMeshMZ3 at fbeed91e; will be archived after this lands.

Sanitizer report (Phase A)
sanitize-history complete:
  commits walked:        26
  prefix auto-prepended: <see logs>
  blank lines inserted:  13
  README.rst->.md fixes: 0
  standalone-guard rm:   0
  blobs reformatted:     21 of 51
  by-kind sniff:         cxx=27 cmake=16 text=7 skip=1

Final history:  26 → 13 commits, 4 → 1 merges

Smaller and more recent upstream than the other v4 ingests so far (PolarTransform, SplitComponents) — fewer style and prefix fixups needed.

Topology and CI expectations
Property Value
Total PR commits 14 (10 rewritten upstream + 4 ITK-side follow-ups)
Merge commits preserved 2
Root commits 1 (unavoidable upstream root — see v4 strategy doc)
Branch base upstream/main @ 777a69ffab (zero commits behind at push time)

Expected CI state: all green except ghostflow-check-main's single root-commit error — accepted per INGESTION_STRATEGY_v4.md §"Known artifacts at PR-review time".

Local verification
  • pre-commit run --all-files → exit 0
  • cmake -DModule_IOMeshMZ3:BOOL=ON . configure clean
  • ninja IOMeshMZ3TestDriver 8/8 build steps OK
  • ninja ITKData to populate fixtures — all 4 CIDs already published on the ITKTestingData GitHub Pages mirror, HTTP 200:
    • bafkreid6pbxwl6yr76kgqjaim3ibqefco5oxdxwzxpamwlyichrmjkf3cu11ScalarMesh.mz3
    • bafkreidhhxis77p7idu2oqko5u3tix665he65kn4yssrweivvtjpbg5yv43Mesh.mz3
    • bafkreib5l2ftbgzhluru2wssotet4l74te3zmmqaj6cr7sl34q4in6cr6iBrainMesh_ICBM152.lh.motor.mz3
    • bafkreieag2mwi2fs5p7lka542fxis2o6tonz5h2v545pnlxtaqvbogwliecortex_5124.mz3
  • ctest -R MZ3: 6/6 tests pass (itkMZ3MeshIOTest1..4, plus KWStyle and InDoxygenGroup)
Phase B (post-merge) plan

Once this PR lands on main:

~/src/ITK-ingest-v4/Utilities/Maintenance/RemoteModuleIngest/archive-remote-module.sh IOMeshMZ3

Phase B publishes the upstream-side removal commit (whitelisted files deleted, MIGRATION_README.md promoted to README.md) directly to InsightSoftwareConsortium/ITKIOMeshMZ3's main and flips archived=true on the GitHub repo settings, per feedback_ingest_archive_readme.md.

@hjmjohnson
Copy link
Copy Markdown
Member Author

@greptileai review this draft before I make it official

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Python wrapping Python bindings for a class 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:Remotes Issues affecting the Remote module labels May 5, 2026
@greptile-apps

This comment was marked as resolved.

Comment thread Modules/IO/IOMeshMZ3/include/itkMZ3MeshIO.h
Comment thread Modules/IO/IOMeshMZ3/include/itkMZ3MeshIOFactory.h
Comment thread Modules/IO/IOMeshMZ3/src/itkMZ3MeshIO.cxx
Comment thread Modules/IO/IOMeshMZ3/src/itkMZ3MeshIO.cxx
Comment thread Modules/IO/IOMeshMZ3/src/itkMZ3MeshIO.cxx
Comment thread Modules/IO/IOMeshMZ3/src/itkMZ3MeshIO.cxx
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 5, 2026
Two upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6214:

1. WritePointData() printed to std::cerr and returned silently when
   m_PointPixelComponentType was UNKNOWNCOMPONENTTYPE, leaving the
   output file in a partially-written state.  Replace with
   itkExceptionMacro so the failure surfaces to the caller.

2. ~MZ3MeshIO() never closed m_Ofstream — a write that threw between
   WriteMeshInformation()'s open and Write()'s explicit close leaked
   the output stream.  Close both m_Ifstream and m_Ofstream in the
   destructor's uncompressed branch (guarded by is_open() so closing
   twice is safe).

Also rewrites the 20 C-style (char *)& casts on gzread / gzwrite /
ifstream / ofstream calls to reinterpret_cast<char *>(&...) per ITK
style guide (Greptile P2 #6).

Also fixes the WriteCells CHAR case that cast the buffer to
unsigned char * instead of char *, which would silently corrupt
negative vertex indices on signed cell-component types (Greptile P2
#5).
Copy link
Copy Markdown
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.

Mostly looks good.

Comment thread Modules/IO/IOMeshMZ3/CMakeLists.txt Outdated
Comment thread Modules/IO/IOMeshMZ3/CMakeLists.txt Outdated
hjmjohnson added 3 commits May 5, 2026 11:40
Brings IOMeshMZ3 from a configure-time remote fetch into the ITK
source tree at Modules/IO/IOMeshMZ3/ using the v4 ingestion
pipeline (whitelist filter-repo + per-commit clang-format + black +
commit-prefix sanitization).

Upstream repo:  https://github.com/InsightSoftwareConsortium/ITKIOMeshMZ3.git
Upstream tip:   fbeed91e6800830e84a7ba5e3461addd98a2e772
Ingest date:    2026-05-05
Whitelist:      IOMeshMZ3.list

Per-commit transforms applied across all 13 commits:
  - filter-repo --paths-from-file (whitelist)
  - filter-repo --to-subdirectory-filter Modules/IO/IOMeshMZ3
  - clang-format -style=file (ITK main's .clang-format) for *.cxx/.h/.hxx/...
  - black for *.py
  - heuristic ITK prefix added to commit subjects without one

Merge topology preserved: 4 -> 1 merge(s).

Primary author: Matt McCormick <matt@mmmccormick.com>

Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu>
hjmjohnson added 5 commits May 5, 2026 11:41
The Phase A sanitizer's text/cmake classifier did not pick this small
file up as 'cmake' (its sniff treated it as 'text'), so gersemi's pass
was skipped.  Apply the formatter now to satisfy ITK's pre-commit
gersemi hook.  Same one-off observed on PR InsightSoftwareConsortium#6215 (IOFDF).
Two upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6214:

1. WritePointData() printed to std::cerr and returned silently when
   m_PointPixelComponentType was UNKNOWNCOMPONENTTYPE, leaving the
   output file in a partially-written state.  Replace with
   itkExceptionMacro so the failure surfaces to the caller.

2. ~MZ3MeshIO() never closed m_Ofstream — a write that threw between
   WriteMeshInformation()'s open and Write()'s explicit close leaked
   the output stream.  Close both m_Ifstream and m_Ofstream in the
   destructor's uncompressed branch (guarded by is_open() so closing
   twice is safe).

Also rewrites the 20 C-style (char *)& casts on gzread / gzwrite /
ifstream / ofstream calls to reinterpret_cast<char *>(&...) per ITK
style guide (Greptile P2 #6).

Also fixes the WriteCells CHAR case that cast the buffer to
unsigned char * instead of char *, which would silently corrupt
negative vertex indices on signed cell-component types (Greptile P2
#5).
- Drop a duplicate consecutive 'protected:' access specifier in
  itkMZ3MeshIO.h (harmless, but warned by some compilers).

- Correct the Doxygen \ingroup tag on itkMZ3MeshIOFactory.h from the
  outdated upstream remote-module name 'ITKIOMeshMZ3' to 'IOMeshMZ3'
  so the factory class shows up under the module's Doxygen group
  alongside MZ3MeshIO.
…shMZ3

In-tree, ITK_SOURCE_DIR is always defined and ITK's top-level
CMakeLists pins the CMake minimum version.  The per-module
cmake_minimum_required line and if(NOT ITK_SOURCE_DIR) ... else()
itk_module_impl() endif() guard are dead code.  The 'set(ITK_DIR
${CMAKE_BINARY_DIR})' line that was inside the in-tree else branch
is preserved (now unconditional) since it actually had effect during
the in-tree build path.  Matches the v4 ingest-pipeline rules in
PR InsightSoftwareConsortium#6204 (sanitize-history.py:patch_drop_cmake_minimum_required and
:patch_standalone_build_guard).
Comment thread Modules/IO/IOMeshMZ3/src/itkMZ3MeshIO.cxx
Symmetric counterpart to the gzopen() failure check in the gzip branch
of the same function: when m_Ofstream.open() fails (target directory
absent, filesystem read-only, permissions), the open silently leaves
the stream in failbit state.  Every subsequent m_Ofstream.write() in
WriteCells / WritePoints / WritePointData / Write() then no-ops, and
Write() closes the stream without surfacing an error — the caller
believes the write succeeded while the output file is empty or absent.

Throw itkExceptionMacro if !m_Ofstream.is_open() after the open call,
matching the existing gzopen guard.  Greptile P1 review on PR InsightSoftwareConsortium#6214
(post-rebase re-review).
@hjmjohnson hjmjohnson merged commit a6863cc into InsightSoftwareConsortium:main May 5, 2026
13 of 17 checks passed
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Two upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6214:

1. WritePointData() printed to std::cerr and returned silently when
   m_PointPixelComponentType was UNKNOWNCOMPONENTTYPE, leaving the
   output file in a partially-written state.  Replace with
   itkExceptionMacro so the failure surfaces to the caller.

2. ~MZ3MeshIO() never closed m_Ofstream — a write that threw between
   WriteMeshInformation()'s open and Write()'s explicit close leaked
   the output stream.  Close both m_Ifstream and m_Ofstream in the
   destructor's uncompressed branch (guarded by is_open() so closing
   twice is safe).

Also rewrites the 20 C-style (char *)& casts on gzread / gzwrite /
ifstream / ofstream calls to reinterpret_cast<char *>(&...) per ITK
style guide (Greptile P2 #6).

Also fixes the WriteCells CHAR case that cast the buffer to
unsigned char * instead of char *, which would silently corrupt
negative vertex indices on signed cell-component types (Greptile P2
#5).
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Symmetric counterpart to the gzopen() failure check in the gzip branch
of the same function: when m_Ofstream.open() fails (target directory
absent, filesystem read-only, permissions), the open silently leaves
the stream in failbit state.  Every subsequent m_Ofstream.write() in
WriteCells / WritePoints / WritePointData / Write() then no-ops, and
Write() closes the stream without surfacing an error — the caller
believes the write succeeded while the output file is empty or absent.

Throw itkExceptionMacro if !m_Ofstream.is_open() after the open call,
matching the existing gzopen guard.  Greptile P1 review on PR InsightSoftwareConsortium#6214
(post-rebase re-review).
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…est-iomeshmz3

ENH: Ingest ITKIOMeshMZ3 into Modules/IO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:IO Issues affecting the IO module area:Python wrapping Python bindings for a class area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation 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

Development

Successfully merging this pull request may close these issues.

3 participants