ENH: Ingest ITKGenericLabelInterpolator into Modules/Filtering

[hjmjohnson]

Brings the GenericLabelInterpolator remote module in-tree at Modules/Filtering/GenericLabelInterpolator/, following the same pattern used for #6093 (AnisotropicDiffusionLBR) and #6103 (Montage). 4-commit stack on top of upstream/main; the upstream history is squashed into the ingest commit so each commit conforms to ITK's commit-quality rules.

Commit stack (top → bottom)

e6e4dc043f  ENH:  Enable GenericLabelInterpolator in CI via configure-ci
46753d110c  COMP: Remove GenericLabelInterpolator.remote.cmake; now in-tree
0e0fd2ce21  COMP: Drop missing README.rst read in ingested GenericLabelInterpolator
68eeb7d281  ENH:  Ingest ITKGenericLabelInterpolator into Modules/Filtering
upstream/main

The squashed ENH: Ingest snapshot is taken from upstream tag v1.3.1 (commit fc78327939). The upstream history (~46 commits, dating to 2014) is preserved at https://github.com/InsightSoftwareConsortium/ITKGenericLabelInterpolator and is cross-referenced from the ingest commit message.

What lives in-tree now

• include/ — 4 headers
  • itkLabelImageGenericInterpolateImageFunction.{h,hxx}
  • itkLabelSelectionImageAdaptor.h
  • itkLabelSelectionPixelAccessor.h
• test/RotateLabels.cxx + 2 .cid content links (already in CID format upstream — no MD5/SHA512 migration needed)
• wrapping/itkLabelImageGenericInterpolateImageFunction.wrap
• Module itk-module.cmake rewritten to drop the file(READ README.rst) call (README.rst is intentionally outside the whitelist; description is now inlined)
• New Modules/Filtering/GenericLabelInterpolator/README.md pointing at the (to-be-archived) upstream

Greptile findings addressed inline in the squashed ingest

• Doxygen \ingroup: removed the stray leading * so doxygen recognises the group tags.
• itk-module.cmake: added explicit ITKImageFunction to DEPENDS (the production header #includes itkInterpolateImageFunction.h); removed the redundant ITKImageFunction from TEST_DEPENDS.
• The "manual int i counter alongside m_Labels iterator" suggestion in the .hxx was deferred — logic is correct, refactor is stylistic, better handled in a follow-up STYLE: PR after this lands.

[greptile-apps]

Greptile Summary

This PR ingests the ITKGenericLabelInterpolator remote module in-tree at Modules/Filtering/GenericLabelInterpolator/, following the same pattern used for AnisotropicDiffusionLBR and Montage. The four-commit stack squashes the upstream history, removes the remote cmake descriptor, enables the module in CI, and applies the targeted correctness fixes surfaced in a prior Greptile pass (Doxygen \ingroup, explicit ITKImageFunction DEPENDS, and a delegating GetRadius() override that correctly propagates higher-order interpolator radii).

Confidence Score: 5/5

Safe to merge; all previously flagged P1 issues are resolved and only minor P2 style nits remain.

No P0 or P1 issues found. The three prior P1 findings (GetRadius(), \ingroup, ITKImageFunction dependency) are all addressed in the squashed ingest commit. Remaining findings are P2: a trailing slash in the test CMakeLists producing a double slash in the written path (benign on all platforms), limited baseline coverage for non-Gaussian interpolator variants, and the acknowledged const_cast on the image adaptor call.

test/CMakeLists.txt (trailing slash on output dir arg) and test/RotateLabels.cxx (only one of seven interpolator paths has a baseline comparison).

Important Files Changed

Filename	Overview
Modules/Filtering/GenericLabelInterpolator/include/itkLabelImageGenericInterpolateImageFunction.h	Main class header: correctly delegates GetRadius() to internal interpolators (fixing the previous P1), proper Doxygen groups, standard ITK type-alias boilerplate.
Modules/Filtering/GenericLabelInterpolator/include/itkLabelImageGenericInterpolateImageFunction.hxx	Implementation: SetInputImage scans labels, builds per-label adaptors/interpolators; EvaluateAtContinuousIndex returns label with highest interpolated weight. Contains const_cast (P2, acknowledged in comment).
Modules/Filtering/GenericLabelInterpolator/itk-module.cmake	Module descriptor with correct DEPENDS (ITKSmoothing, ITKImageAdaptors, ITKImageFunction explicit), TEST_DEPENDS, EXCLUDE_FROM_DEFAULT, ENABLE_SHARED — addresses previous ITKImageFunction dependency finding.
Modules/Filtering/GenericLabelInterpolator/test/CMakeLists.txt	Test driver setup and single itk_add_test; trailing slash on output dir arg leads to double slash in written path (P2); only one of seven interpolator outputs is baseline-compared.
Modules/Filtering/GenericLabelInterpolator/test/RotateLabels.cxx	Rotation torture test for seven interpolator variants; only the Gaussian-with-3-rotations output is validated against a baseline. Core test logic is correct.
Modules/Filtering/GenericLabelInterpolator/wrapping/itkLabelImageGenericInterpolateImageFunction.wrap	Wraps Linear and NearestNeighbor variants for all scalar+UC+colour types; vector types intentionally commented out due to upstream operator< limitation.
Modules/Remote/GenericLabelInterpolator.remote.cmake	Remote module descriptor correctly removed now that the module is in-tree.
pyproject.toml	Adds -DModule_GenericLabelInterpolator:BOOL=ON to the CI configure command, consistent with AnisotropicDiffusionLBR and Montage entries.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class InterpolateImageFunction {
        +EvaluateAtContinuousIndex()
        +GetRadius() SizeType
        +SetInputImage()
    }
    class LabelImageGenericInterpolateImageFunction {
        -m_Labels : LabelSetType
        -m_InternalInterpolators : vector
        -m_LabelSelectionAdaptors : vector
        +SetInputImage(image)
        +EvaluateAtContinuousIndex(cindex) OutputType
        +GetRadius() SizeType
    }
    class LabelSelectionImageAdaptor {
        +SetAcceptedValue(value)
    }
    class ImageAdaptor {
        +SetImage(image)
    }
    class LabelSelectionPixelAccessor {
        -m_AcceptedValue : TInternalType
        +Get(input) TExternalType
        +SetAcceptedValue(value)
    }
    class TInterpolator {
        <<template>>
        +EvaluateAtContinuousIndex()
        +GetRadius() SizeType
        +SetInputImage()
    }
    InterpolateImageFunction <|-- LabelImageGenericInterpolateImageFunction
    ImageAdaptor <|-- LabelSelectionImageAdaptor
    LabelSelectionImageAdaptor --> LabelSelectionPixelAccessor : uses accessor
    LabelImageGenericInterpolateImageFunction --> LabelSelectionImageAdaptor : 1 per label
    LabelImageGenericInterpolateImageFunction --> TInterpolator : 1 per label
    TInterpolator --> LabelSelectionImageAdaptor : SetInputImage(adaptor)

Loading

_{Reviews (2): Last reviewed commit: "BUG: Delegate GetRadius() to the wrapped..." | Re-trigger Greptile}

[hjmjohnson]

@greptileai review

Re-review on new HEAD 5b5cc6c3ec. Adds BUG: Delegate GetRadius() to the wrapped per-label interpolator on top of the previously-reviewed squashed ingest. Earlier P2 findings (Doxygen \ingroup, ITKImageFunction DEPENDS) are addressed in the squashed commit; the range-for refactor was deferred.

[hjmjohnson]

@dzenanz The ghostflow issues would require changing the commit history. I chose not to do that for the ingestions.

[blowekamp]

@hjmjohnson I don't think it's worth changing the commit messages. But I do see there are some white space issues in the history. Extra white space and EOF not where expected.

Do the current file pass these white space tests?

It may be reasonable to automate a fix up the commit history for these whitespace errors.

[dzenanz]

We can ignore ghostflow in this case. Brad's idea to remove trailing whitespaces sounds good to me.

[hjmjohnson]

@dzenanz @blowekamp, cleaning up the whitespace and cleaning up the commit messages are both really easy and quite safe to do. As I move forward with ingesting other remote modules, it is a really easy thing to add, and it will make reviewing the PR's easier.

The reason I didn't do that was to preserve the original code, but now that I think about it, I think the benefits of byte-for-byte preservation are probably a vanishingly small benefit.

[dzenanz]

Did the trailing whitespaces get removed by the merge commit (1ee0cb1)? That is fine, too.

[dzenanz]

Mostly looks good.

[hjmjohnson]

@blowekamp @dzenanz — applied the per-commit cleanup we discussed. Summary on new HEAD 4db9b1e93e:

• Rebased onto current upstream/main (was 18 behind).
• Re-stamped every historical commit through ITK's current pre-commit hook set so git show <sha> reads cleanly all the way back to the 2016 commits. Intermediate trailing-whitespace / EOF / formatting drift is gone.
• Subject lines that violated ITK conventions (no prefix, or > 78 chars) were rewritten and the original subject preserved verbatim in the body as Original subject: <text>. 4 of 41 commits were affected.
• 2 intermediate style-only commits (STYLE: Update to match clang-format-19 and STYLE: gersemi formatting) became empty after the replay and were dropped — they would not have existed under today's hook set in the first place.
• Author name, email, and author-date are preserved verbatim on every commit. All 12 original contributors still appear in git shortlog -sne (unchanged set). Tip tree is byte-identical to the pre-rewrite tip (zero CI behavior delta).

The driver landed as a new commit at the tip (ENH: Per-commit pre-commit replay for remote-module ingestion) and is now documented in Utilities/Maintenance/RemoteModuleIngest/INGESTION_STRATEGY.md as the default pass for modes A/B going forward.

Verification commands

# Tip tree unchanged
git diff pre-normalize-6135..HEAD                         # empty

# Authorship preserved (compare against the pre-rewrite branch)
git log --format='%an <%ae>' upstream/main..HEAD | sort -u   # 12 distinct authors

# Every historical commit passes modern pre-commit
for sha in $(git rev-list upstream/main..HEAD); do
    git stash && git checkout $sha
    pre-commit run --files Modules/Filtering/GenericLabelInterpolator/**/* \
        || echo "DIRTY: $sha"
    git checkout - && git stash pop
done                                                       # no DIRTY lines

[hjmjohnson]

@blowekamp @dzenanz — followup on 68a9eb30c3: ghostflow flagged the original first commit (7a7bd8331a, 2016) for trailing whitespace it had added, and the per-commit pre-commit replay didn't catch it on the first pass. Re-ran the normalize script (idempotent), it now correctly cleared all trailing whitespace from every historical commit. Verified with git show <sha> --check across every commit in upstream/main..HEAD — no trailing-whitespace warnings remain. One additional commit (ENH: Add .gitattributes…) became empty after the second pass and was dropped, taking the count from 40 to 39.

[dzenanz]

I just took a look. All the commit now live on top of the main branch. Which is different from the other ingested modules, and feels strange. I assume it would be confusing to someone in the future reading the log, unaware of this recent ingestion campaign.

[hjmjohnson]

For reviewers: ghostflow on this PR shows exactly one error and that is intentional and required:

- commit f30c6ba6ca50141139c82184dec00bb606449618 not allowed; it is a root commit.

This is the structural signature of the unrelated-history merge — git merge --allow-unrelated-histories --no-ff requires that the second parent of the merge commit have no common ancestor with main, which forces the side branch's tail to be a root commit. Ghostflow's "no root commits" rule is a sensible default for normal contributions but is structurally incompatible with the merge-commit topology required for remote-module ingestion.

Codified in Utilities/Maintenance/RemoteModuleIngest/INGESTION_STRATEGY.md (commit 957dae26f4) under "The root-commit ghostflow error is REQUIRED, not a failure" with explicit acceptance criteria:

1. Exactly one ghostflow error
2. The error is the "root commit not allowed" form
3. The flagged SHA equals the side branch's tail commit (git rev-list --max-parents=0 <merge-commit>^2)

Anything beyond that single root-commit error (e.g., trailing-whitespace or missing-EOF errors) is a real failure that must be fixed before merge — those would indicate either intermediate-commit drift or a side-branch tree that wasn't subtree-only.

f30c6ba6ca here is the side branch's root commit (Francois Budin's 2016 "Converting generic_label_interpolator to an ITK remote module"), exactly as required.

[dzenanz]

Looks great now!