ENH: Ingest ITKIOFDF into Modules/IO#6215
Merged
hjmjohnson merged 65 commits intoInsightSoftwareConsortium:mainfrom May 5, 2026
Merged
ENH: Ingest ITKIOFDF into Modules/IO#6215hjmjohnson merged 65 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson merged 65 commits intoInsightSoftwareConsortium:mainfrom
Conversation
FDF files seem to have 3D rotation matrices even if the image is 2D. This can give them degenerate directions cosines. If that's the case punt and use identity direction cosines.
This method is expected and with out it a link error is generated.
Add register and export
The modules name is inconsistent and causing not to be used as a module in ITK. Previously ITK was calling itk FDFImageIO, while internally it was called ITKFDFImageIO. Neither name followed ITK conventions. The correct name for an external module does not begin with the ITK prefix and for this case should be: IOFDF The name of the module and related variable, libraries, export specifications etc, have been rename consistently.
BUG: Rename Module to IOFDF
fix wrapping module name
Exclude from default. When this remote module was already downloaded, it was automatically turned enabled by default. Remove modules should not be enable by default
Moved "common" code to itk namespace. Fixed variable alignment and header guarders.
STYLE: fixes according to KWStyle
Set the required doxygen ingroup.
BUG: Fix IOFDFInDoxygenGroup test
BUG: Correct test function signature for windows
Suggested-by: Hans Johnson <hans-johnson@uiowa.edu>
Add Hans' style fixes and remove old internal copy.
Assign copyright to the Insight Software Consortium and use the Apache 2 license, ITK's current license. This will be merged pending approval by Glenn Pierce.
ENH: Update the license header to the ISC copyright.
This commit updates CMakeLists.txt to ensure built library is shared. It also uses "RegisterFactoryInternal" to make sure the factory is registered once into the ImageIOFactory. See InsightSoftwareConsortium/ITK@39b4ab3 (BUG: Internal factory must use RegisterFactoryInternal method) for more details. See ITK issue InsightSoftwareConsortium#3393 https://issues.itk.org/jira/browse/ITK-3393
BUG: Ensure factory is registered once.
STYLE: Use Macro for Function Deletion
This new parameter allows ITK to be aware any object that needs to be registered to the ITK factory. Remote modules that offer IO capabilities can be more easily integrated in ITK.
ENH: Add FACTORY_NAMES parameter to itk_module() declaration
Require CMake minimum version to be 3.9.5 following ITKv5 requiring C++11: https://discourse.itk.org/t/minimum-cmake-version-update/585
…mumVersio n ENH: Require cmake minimum version to be 3.9.5.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 5, 2026
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).
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 5, 2026
Three upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6215: 1. CanReadFile threw an ITK ExceptionObject when the file couldn't be opened. ITK's ImageIOFactory contract requires CanReadFile to return false for unreadable files; throwing aborts every probe of non-FDF files in itk::ImageIOFactory::CreateImageIO. Replaced the throw with a return false. 2. ReadImageInformation() iterated 'i < GetNumberOfDimensions()' and indexed m_Roi[i] without checking m_Roi.size(). Files lacking a 'roi' header field would silently read out-of-bounds. Added an itkExceptionMacro guard that surfaces the missing entries. 3. itk-module.cmake DEPENDS listed ITKNIFTI as a copy-paste artefact; no FDF source includes any NIFTI header or uses any NIFTI type. Removed the spurious dependency.
dzenanz
approved these changes
May 5, 2026
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
Mostly looks good on a glance.
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 5, 2026
Upstream remote modules typically begin their top-level CMakeLists.txt with their own cmake_minimum_required(VERSION X.Y.Z) declaration, often 3.10.2 or earlier. ITK's top-level CMakeLists pins a higher minimum, so the per-module line is redundant and frequently *lower* than ITK's floor. @dzenanz flagged it on every ingest with the comment 'Version behind, not needed.' — most recently on PR InsightSoftwareConsortium#6215 (IOFDF). Add patch_drop_cmake_minimum_required() that detects the idiom and removes the matching line via a multiline regex. Track count via sanitizer.cmake_min_required_drops and print it in the run summary. Apply alongside patch_standalone_build_guard() in the cmake-blob branch of blob_callback.
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 5, 2026
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).
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 5, 2026
Three upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6215: 1. CanReadFile threw an ITK ExceptionObject when the file couldn't be opened. ITK's ImageIOFactory contract requires CanReadFile to return false for unreadable files; throwing aborts every probe of non-FDF files in itk::ImageIOFactory::CreateImageIO. Replaced the throw with a return false. 2. ReadImageInformation() iterated 'i < GetNumberOfDimensions()' and indexed m_Roi[i] without checking m_Roi.size(). Files lacking a 'roi' header field would silently read out-of-bounds. Added an itkExceptionMacro guard that surfaces the missing entries. 3. itk-module.cmake DEPENDS listed ITKNIFTI as a copy-paste artefact; no FDF source includes any NIFTI header or uses any NIFTI type. Removed the spurious dependency.
Brings IOFDF from a configure-time remote fetch into the ITK source tree at Modules/IO/IOFDF/ using the v4 ingestion pipeline (whitelist filter-repo + per-commit clang-format + black + commit-prefix sanitization). Upstream repo: https://github.com/InsightSoftwareConsortium/ITKIOFDF.git Upstream tip: 3a3ff84c803d4226176078d32ddd0760c71a5b2c Ingest date: 2026-05-05 Whitelist: IOFDF.list Per-commit transforms applied across all 55 commits: - filter-repo --paths-from-file (whitelist) - filter-repo --to-subdirectory-filter Modules/IO/IOFDF - 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: 32 -> 14 merge(s). Primary author: Hans Johnson <hans-johnson@uiowa.edu> Co-authored-by: Bradley Lowekamp <blowekamp@mail.nih.gov> Co-authored-by: Dženan Zukić <dzenan.zukic@kitware.com> Co-authored-by: Francois Budin <francois.budin@kitware.com> Co-authored-by: Gaëtan Lehmann <gaetan.lehmann@gmail.com> Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com> Co-authored-by: Jon Haitz Legarreta <jhlegarreta@vicomtech.org> Co-authored-by: Jon Haitz Legarreta Gorroño <jon.haitz.legarreta@gmail.com> Co-authored-by: Kent Williams <norman-k-williams@uiowa.edu> Co-authored-by: Mathew Seng <mathewseng@gmail.com> Co-authored-by: Matt McCormick <matt.mccormick@kitware.com> Co-authored-by: Matt McCormick <matt@mmmccormick.com> Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com> Co-authored-by: Zach Williamson <zachary-williamson@uiowa.edu>
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.
Three upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6215: 1. CanReadFile threw an ITK ExceptionObject when the file couldn't be opened. ITK's ImageIOFactory contract requires CanReadFile to return false for unreadable files; throwing aborts every probe of non-FDF files in itk::ImageIOFactory::CreateImageIO. Replaced the throw with a return false. 2. ReadImageInformation() iterated 'i < GetNumberOfDimensions()' and indexed m_Roi[i] without checking m_Roi.size(). Files lacking a 'roi' header field would silently read out-of-bounds. Added an itkExceptionMacro guard that surfaces the missing entries. 3. itk-module.cmake DEPENDS listed ITKNIFTI as a copy-paste artefact; no FDF source includes any NIFTI header or uses any NIFTI type. Removed the spurious dependency.
Not listed in src/CMakeLists.txt (so the build is unaffected by its presence), but carries a hardcoded /home/glenn absolute path, imports VTK headers the IOFDF module does not declare as dependencies, and runs an infinite while(1) viewer->Render() loop. Pure noise from the upstream remote module's developer environment.
itkFDFImageIO.h had \ingroup IOIOFDF (typo), itkFDFImageIOFactory.h had \ingroup ITKIOFDF (the old upstream remote-module name). The in-tree module is named 'IOFDF', so both must use \ingroup IOFDF for the classes to appear in the module's Doxygen group.
The line cmake_minimum_required(VERSION 3.10.2) survived the upstream ingest. ITK's top-level CMakeLists already pins a higher minimum (and the project's policy is that ingested module CMakeLists do not declare their own minimum), so the per-module line is dead code. Address @dzenanz review comment.
ReadImageInformation() guarded its parsing block on tokens.size() == 4, which silently drops every header line whose value contains a space — notably 'unsigned char' / 'unsigned short' / 'unsigned int' / 'unsigned long' storage types. ParseLine strips the FDF C-style decoration and Tokenize splits on ' ;', producing five tokens for those values; the == 4 check then filtered them out and the value-comparison branches that handled them became unreachable. Loosen the guard to tokens.size() >= 4 and re-join tokens[3..N-1] into a single space-separated value so the existing 'unsigned char' / 'unsigned short' / etc. branches actually fire. Header lines with multi-word vector values (e.g. orientation, span, origin, roi) were already handled correctly because their parsers split on ',' / StringToVector, which tokenize the value internally; the only behaviour change is that previously-skipped scalar 'unsigned X' storage types now reach the SetComponentType dispatch.
hjmjohnson
merged commit ce991b3
into
InsightSoftwareConsortium:main
14 of 17 checks passed
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 6, 2026
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).
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 6, 2026
Three upstream bugs caught by Greptile review on PR InsightSoftwareConsortium#6215: 1. CanReadFile threw an ITK ExceptionObject when the file couldn't be opened. ITK's ImageIOFactory contract requires CanReadFile to return false for unreadable files; throwing aborts every probe of non-FDF files in itk::ImageIOFactory::CreateImageIO. Replaced the throw with a return false. 2. ReadImageInformation() iterated 'i < GetNumberOfDimensions()' and indexed m_Roi[i] without checking m_Roi.size(). Files lacking a 'roi' header field would silently read out-of-bounds. Added an itkExceptionMacro guard that surfaces the missing entries. 3. itk-module.cmake DEPENDS listed ITKNIFTI as a copy-paste artefact; no FDF source includes any NIFTI header or uses any NIFTI type. Removed the spurious dependency.
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 6, 2026
…est-iofdf ENH: Ingest ITKIOFDF into Modules/IO
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 7, 2026
Upstream remote modules typically begin their top-level CMakeLists.txt with their own cmake_minimum_required(VERSION X.Y.Z) declaration, often 3.10.2 or earlier. ITK's top-level CMakeLists pins a higher minimum, so the per-module line is redundant and frequently *lower* than ITK's floor. @dzenanz flagged it on every ingest with the comment 'Version behind, not needed.' — most recently on PR InsightSoftwareConsortium#6215 (IOFDF). Add patch_drop_cmake_minimum_required() that detects the idiom and removes the matching line via a multiline regex. Track count via sanitizer.cmake_min_required_drops and print it in the run summary. Apply alongside patch_standalone_build_guard() in the cmake-blob branch of blob_callback.
50 tasks
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
10 participants
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.
Ingests ITKIOFDF (read support for the FDF flexible-data-format files produced by Varian / Agilent MR scanners —
itk::FDFImageIO+ factory) intoModules/IO/IOFDF/using the v4 ingestion pipeline. Tier-A Wave-1 candidate from #6160. Upstream: InsightSoftwareConsortium/ITKIOFDF at3a3ff84c; will be archived after this lands.Sanitizer report (Phase A)
One small post-Phase-A
STYLE:follow-up was needed:Modules/IO/IOFDF/src/CMakeLists.txt(3 lines) was sniffed astextrather thancmakeby the sanitizer's content classifier and bypassed gersemi. Manual gersemi-format applied in commitfc131e87. Future refinement tosanitize-history.py:sniff_kind()could force any path basename matchingCMakeLists.txtor*.cmaketokind=cmakeregardless of content sniff.Topology and CI expectations
upstream/main@777a69ffab(zero commits behind at push time)Expected CI state: all green except
ghostflow-check-main's single root-commit error — accepted perINGESTION_STRATEGY_v4.md§"Known artifacts at PR-review time".Local verification
pre-commit run --all-files→ exit 0cmake -DModule_IOFDF:BOOL=ON .configure cleanninja IOFDFTestDriver9/9 build steps OKctest -R IOFDF: 3/3 tests pass (no ExternalData fetches required)IOFDFKWStyleTestIOFDFInDoxygenGroupitkFDFImageIOTestPhase B (post-merge) plan
Once this PR lands on
main:~/src/ITK-ingest-v4/Utilities/Maintenance/RemoteModuleIngest/archive-remote-module.sh IOFDFPhase B publishes the upstream-side removal commit (whitelisted files deleted,
MIGRATION_README.mdpromoted toREADME.md) directly toInsightSoftwareConsortium/ITKIOFDF'smainand flipsarchived=trueon the GitHub repo settings, perfeedback_ingest_archive_readme.md.