ENH: Ingest ITKSplitComponents into Modules/Filtering#6212
Merged
hjmjohnson merged 67 commits intoInsightSoftwareConsortium:mainfrom May 5, 2026
Merged
ENH: Ingest ITKSplitComponents into Modules/Filtering#6212hjmjohnson merged 67 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson merged 67 commits intoInsightSoftwareConsortium:mainfrom
Conversation
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.
BUG: Add EXCLUDE_FROM_DEFAULT.
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.
ENH: Providing ITKv5 updates for C++11
Use constexpr for constant numeric literals.
ENH: Providing ITKv5 updates for C++11
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.
…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).
This comment was marked as resolved.
This comment was marked as resolved.
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.
Member
|
Looks good on a glance. |
dzenanz
approved these changes
May 5, 2026
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).
6dc5a8b
into
InsightSoftwareConsortium:main
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
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
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 ITKSplitComponents (header-only
itk::SplitComponentsImageFilter— splits a vector / covariant-vector / symmetric-tensor image into per-component scalar images) intoModules/Filtering/SplitComponents/using the v4 ingestion pipeline. Tier-A Wave-1 candidate from #6160. Upstream: InsightSoftwareConsortium/ITKSplitComponents at94fde65c; will be archived after this lands.Sanitizer report (Phase A)
The v4 sanitizer's
standalone-guard rmrule fired 3 times, auto-unwrapping the upstreamif(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
upstream/main@777a69ffab(zero commits behind at push time)Expected CI state on push: all green except
ghostflow-check-main, which fails with the singlecommit <sha> not allowed; it is a root commiterror. That single error is documented as expected and accepted for every v4 ingest inINGESTION_STRATEGY_v4.md§"Known artifacts at PR-review time" (PR #6204).Local verification
pre-commit run --all-files→ exit 0cmake -DModule_SplitComponents:BOOL=ON .configure cleanninja SplitComponentsTestDriverbuild OKninja ITKDatato populate baselines (bothbafkreiamxu.../bafkreif3tk...CIDs already published on the ITKTestingData GitHub Pages mirror, HTTP 200)ctest -R SplitComponents: 3/3 tests passSplitComponentsKWStyleTestSplitComponentsInDoxygenGroupitkSplitComponentsImageFilterTest(with--comparebaseline match)Phase B (post-merge) plan
Once this PR lands on
main:~/src/ITK-ingest-v4/Utilities/Maintenance/RemoteModuleIngest/archive-remote-module.sh SplitComponentsPhase B publishes the upstream-side removal commit (whitelisted files deleted,
MIGRATION_README.mdpromoted toREADME.md) directly toInsightSoftwareConsortium/ITKSplitComponents'smainand flipsarchived=trueon the GitHub repo settings, perfeedback_ingest_archive_readme.md.