Skip to content

ENH: Ingest ITKSplitComponents into Modules/Filtering#6212

Merged
hjmjohnson merged 67 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-splitcomponents
May 5, 2026
Merged

ENH: Ingest ITKSplitComponents into Modules/Filtering#6212
hjmjohnson merged 67 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-splitcomponents

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Ingests ITKSplitComponents (header-only itk::SplitComponentsImageFilter — splits a vector / covariant-vector / symmetric-tensor image into per-component scalar images) into Modules/Filtering/SplitComponents/ using the v4 ingestion pipeline. Tier-A Wave-1 candidate from #6160. Upstream: InsightSoftwareConsortium/ITKSplitComponents at 94fde65c; will be archived after this lands.

Sanitizer report (Phase A)
sanitize-history complete:
  commits walked:        198
  prefix auto-prepended: <see logs>
  standalone-guard rm:   3
  blobs reformatted:     49 of 53
  by-kind sniff:         cxx=40 cmake=22 text=4 skip=2

Final history:  198 → 61 commits, 61 → 19 merges

The v4 sanitizer's standalone-guard rm rule fired 3 times, auto-unwrapping the upstream if(NOT ITK_SOURCE_DIR) ... else() itk_module_impl() endif() standalone-build wrapper across the module's CMake history. No post-PR fixups needed for that pattern.

Topology and CI expectations
Property Value
Total PR commits 62 (58 rewritten upstream + 4 ITK-side follow-ups)
Merge commits preserved 20
Root commits 1 (unavoidable upstream root — see v4 strategy doc, "Known artifacts at PR-review time")
Branch base upstream/main @ 777a69ffab (zero commits behind at push time)

Expected CI state on push: all green except ghostflow-check-main, which fails with the single commit <sha> not allowed; it is a root commit error. That single error is documented as expected and accepted for every v4 ingest in INGESTION_STRATEGY_v4.md §"Known artifacts at PR-review time" (PR #6204).

Local verification
  • pre-commit run --all-files → exit 0
  • cmake -DModule_SplitComponents:BOOL=ON . configure clean
  • ninja SplitComponentsTestDriver build OK
  • ninja ITKData to populate baselines (both bafkreiamxu... / bafkreif3tk... CIDs already published on the ITKTestingData GitHub Pages mirror, HTTP 200)
  • ctest -R SplitComponents: 3/3 tests pass
    • SplitComponentsKWStyleTest
    • SplitComponentsInDoxygenGroup
    • itkSplitComponentsImageFilterTest (with --compare baseline match)
Phase B (post-merge) plan

Once this PR lands on main:

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

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

thewtex and others added 30 commits May 13, 2010 10:57
People will not look for TensorComponents.
This allows one to specify which output components are allocated and populated.
Based off feedback from Cory Quammen in review comments in the Insight Journal.
These are reserved for compiler pre-processor definitions.
STYLE: Use Macro for Function Deletion
Provide initial conversion of features to ITKv5.
Provide remove virtual and override

Use clang-tidy to add ITK_OVERRIDE, and to remove
redundant virtual on functions.

cd ../ITK;
clang-tidy -p ITK-clangtidy $find Modules/[A-J]*  -name *.cxx |fgrep -v ThirdParty) -checks=-*,modernize-use-override  -header-filter=.* -fix
clang-tidy -p ITK-clangtidy $(find Modules/[K-Z]*  -name *.cxx |fgrep -v ThirdParty) -checks=-*,modernize-use-override  -header-filter=.* -fix

https://stackoverflow.com/questions/39932391/virtual-override-or-both-c

When you override a function you don't technically need to write either virtual
or override.

The original base class declaration needs the keyword virtual to mark it as virtual.

In the derived class the function is virtual by way of having the ¹same type as
the base class function.

However, an override can help avoid bugs by producing a compilation error when
the intended override isn't technically an override. E.g. that the function
type isn't exactly like the base class function. Or that a maintenance of the
base class changes that function's type, e.g. adding a defaulted argument.

In the same way, a virtual keyword in the derived class can make such a bug
more subtle, by ensuring that the function is still is virtual in further
derived classes.

So the general advice is,

virtual for the base class function declaration.
This is technically necessary.

Use override (only) for a derived class' override.
This helps with maintenance.
git grep -l "ITK_OVERRIDE" |   fgrep -v itk_compiler_detection.h | fgrep -v CMakeLists.txt |fgrep -v .cmake |   xargs sed -i '' -e "s/ITK_OVERRIDE/override/g"
ENH: Providing ITKv5 updates for C++11
Use static constexpr directly now that C++11 conformance
is required by all compilers.

:%s/itkStaticConstMacro *( *\([^,]*\),[ \_s]*\([^,]*\),\_s*\([^)]*\)) */static constexpr \2 \1 = \3/ge
== http://en.cppreference.com/w/cpp/language/type_alias ==

Type alias is a name that refers to a previously defined type (similar
to typedef).

A type alias declaration introduces a name which can be used as a
synonym for the type denoted by type-id. It does not introduce a new
type and it cannot change the meaning of an existing type name. There is
no difference between a type alias declaration and typedef declaration.
This declaration may appear in block scope, class scope, or namespace
scope.

== https://www.quora.com/Is-using-typedef-in-C++-considered-a-bad-practice ==

While typedef is still available for backward compatibility, the new
Type Alias syntax 'using Alias = ExistingLongName;' is more consistent
with the flow of C++ than the old typedef syntax 'typedef
ExistingLongName Alias;', and it also works for templates (Type alias,
alias template (since C++11)), so leftover 'typedef' aliases will differ
in style from any alias templates.
Use constexpr for constant numeric literals.
Move `ITK_DISALLOW_COPY_AND_ASSIGN` calls to public section following
the discussion in
https://discourse.itk.org/t/noncopyable

If legacy (pre-macro) copy and assing methods existed, subsitute them
for the `ITK_DISALLOW_COPY_AND_ASSIGN` macro.
…ToPublicS

ection

COMP: Move ITK_DISALLOW_COPY_AND_ASSIGN calls to public section.
Fix errors steming from the use of the new `itk::PoolMultiThreader`
class for multi-threading.

The errors stemmed when this gerrit topic got merged:
http://review.source.kitware.com/#/c/23175/

And were later related to the following gerrit-topic:
http://review.source.kitware.com/#/c/23434/

The solution adopted in this patch set was based on the proposal in:
http://review.source.kitware.com/#/c/23439/
BUG: Fix test errors steming from the new multi-threading mechanism.
…ivar.

Prefer the boolean macro function call to initialize the dynamic
multi-threading ivar for the reasons stated here:
http://review.source.kitware.com/#/c/23434/
Re-work the `CMakeLists.txt` file so that the module can be built
outside ITK, following:
InsightSoftwareConsortium/ITKModuleTemplate@b4d185d
…DirectMul

tiThreadingIvarInitialization

BUG: Use boolean macro call to initialize dynamic multi-threading ivar.
tbirdso and others added 13 commits June 2, 2022 09:13
…ftwareConsortium/bump_and_format

ENH: Bump ITK and change http to https
Resolves issues identified in automated Python notebook testing:
- The number of components to split with `SplitComponentsImageFilter`
  is now fixed at 3 for RGB and 4 for RGBA images rather than being
  defined by the image dimension. Under previous behavior only the
   first two RG components could be extracted for a 2D image.
- Updated to use pythonic `itk.image_duplicator`
- Downgrades `traitlets` for compatibility with `itkwidgets==0.32.5`

Also updates `.gitignore` to ignore example artifacts.
…t-notebook

ENH: Add ARM builds and notebook test
Resolves CMake warning regarding CMP135 policy introduced in CMake 3.24
…p-workflow

ENH: Bump reusable workflow for wheel validation
…acro

Added two new macro's, intended to replace the old 'itkTypeMacro' and
'itkTypeMacroNoParent'.

The main aim is to be clearer about what those macro's do: add a virtual
'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro',
'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it
was not used anyway.

Note that originally 'itkTypeMacro' did not use its 'superclass' parameter
either, looking at commit 699b66c, Will
Schroeder, June 27, 2001:
https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
Remove ITK prefix from variable names as it's for internal ITK modules.
Use correct SplitComponents-Test_LIBRARIES instead of ITKSplitComponents-Test_LIBRARIES.
Update test driver name and related variables accordingly.
Brings SplitComponents from a configure-time remote fetch into the ITK
source tree at Modules/Filtering/SplitComponents/ using the v4 ingestion
pipeline (whitelist filter-repo + per-commit clang-format + black +
commit-prefix sanitization).

Upstream repo:  https://github.com/InsightSoftwareConsortium/ITKSplitComponents.git
Upstream tip:   94fde65cae6883c9a0f1508c37c501c2ad989cdd
Ingest date:    2026-05-05
Whitelist:      SplitComponents.list

Per-commit transforms applied across all 61 commits:
  - filter-repo --paths-from-file (whitelist)
  - filter-repo --to-subdirectory-filter Modules/Filtering/SplitComponents
  - 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: 61 -> 19 merge(s).

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

Co-authored-by: Bradley Lowekamp <blowekamp@mail.nih.gov>
Co-authored-by: Brian Helba <brian.helba@kitware.com>
Co-authored-by: Dženan Zukić <dzenan.zukic@kitware.com>
Co-authored-by: Francois Budin <francois.budin@kitware.com>
Co-authored-by: Hans J. Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hans Johnson <hans.j.johnson@gmail.com>
Co-authored-by: Jon Haitz Legarreta <jhlegarreta@vicomtech.org>
Co-authored-by: Jon Haitz Legarreta Gorroño <jhlegarreta@vicomtech.org>
Co-authored-by: Jon Haitz Legarreta Gorroño <jon.haitz.legarreta@gmail.com>
Co-authored-by: Mathew Seng <mathewseng@gmail.com>
Co-authored-by: Matt McCormick (thewtex) <matt@mmmccormick.com>
Co-authored-by: Matt McCormick <matt.mccormick@kitware.com>
Co-authored-by: Tom Birdsong <40648863+tbirdso@users.noreply.github.com>
Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com>
Co-authored-by: Zach Williamson <zachary-williamson@uiowa.edu>
The module now lives under Modules/Filtering/SplitComponents/.  Drop
the configure-time fetch entry; consumers enable the module via
Module_SplitComponents=ON like any other in-tree module.
Now that SplitComponents is in-tree under Modules/Filtering/SplitComponents/,
exercise it on every pixi configure-ci run (Linux / macOS / Windows
GitHub Actions matrix).
@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:Filtering Issues affecting the Filtering module area:Remotes Issues affecting the Remote module labels May 5, 2026
@greptile-apps

This comment was marked as resolved.

Comment thread Modules/Filtering/SplitComponents/wrapping/itkSplitComponentsImageFilter.wrap Outdated
Two upstream wrapping bugs caught by Greptile review on PR InsightSoftwareConsortium#6212:

1. CovariantVector wrap block guarded on ITKT_IV<t><d><d> (Vector
   image type prefix) instead of ITKT_ICV<t><d><d> (CovariantVector
   image type prefix).  In a configuration with
   ITK_WRAP_covariant_vector_float=OFF and ITK_WRAP_vector_float=ON
   this would emit dangling itk_wrap_template calls referencing
   undefined CovariantVector types.

2. Removed two redundant itk_wrap_image_filter_types(...) calls that
   followed the per-dimension foreach loops in the
   ITK_WRAP_rgb_unsigned_short and ITK_WRAP_rgba_unsigned_char
   branches.  The macros iterate over all enabled dimensions, so the
   trailing call duplicated registrations.  The asymmetric structure
   (only those two branches had the duplicate; rgb_unsigned_char and
   rgba_unsigned_short already lacked it) confirms a copy-paste
   artifact upstream.
@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented May 5, 2026

Looks good on a glance.

In-tree, ITK's top-level CMakeLists pins the CMake minimum, so the
per-module declaration is dead code.  Matches the v4 ingest-pipeline
rule in PR InsightSoftwareConsortium#6204 (sanitize-history.py:patch_drop_cmake_minimum_required).
@hjmjohnson hjmjohnson added this to the ITK 6.0 Release Candidate 1 milestone May 5, 2026
@hjmjohnson hjmjohnson merged commit 6dc5a8b into InsightSoftwareConsortium:main May 5, 2026
11 of 16 checks passed
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Two upstream wrapping bugs caught by Greptile review on PR InsightSoftwareConsortium#6212:

1. CovariantVector wrap block guarded on ITKT_IV<t><d><d> (Vector
   image type prefix) instead of ITKT_ICV<t><d><d> (CovariantVector
   image type prefix).  In a configuration with
   ITK_WRAP_covariant_vector_float=OFF and ITK_WRAP_vector_float=ON
   this would emit dangling itk_wrap_template calls referencing
   undefined CovariantVector types.

2. Removed two redundant itk_wrap_image_filter_types(...) calls that
   followed the per-dimension foreach loops in the
   ITK_WRAP_rgb_unsigned_short and ITK_WRAP_rgba_unsigned_char
   branches.  The macros iterate over all enabled dimensions, so the
   trailing call duplicated registrations.  The asymmetric structure
   (only those two branches had the duplicate; rgb_unsigned_char and
   rgba_unsigned_short already lacked it) confirms a copy-paste
   artifact upstream.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…est-splitcomponents

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

Labels

area:Filtering Issues affecting the Filtering 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.

9 participants