WIP: Use itk.IT as IdentifierType in MeshArrayPixelTypeTest

[N-Dekker]

itk.IT is defined as platform-independent alias for IdentifierType.

• Follow-up to pull request COMP: Use itk.ULL in Windows Platform #2970 by @PranjalSahu

[N-Dekker]

Instead of introducing this IdentifierType alias, we might as well directly use itk.IT, as in:

v = itk.VectorContainer[itk.IT, PixelType].New()

But I guess IdentifierType is more readable than itk.IT 🤷

[greptile-apps]

Greptile Summary

This change replaces the mesh test's runtime point-data container type selection with the itk.IT alias. A focused validation confirmed that this alias selects a container key that differs from the wrapped mesh identifier type on Windows builds configured without 64-bit IDs, preventing the test from passing its point data to SetPointData.

Confidence Score: 4/5

Not safe to merge until the Windows identifier-type selection is made consistent with the wrapped mesh type.

The focused validation exercised the configured type-selection path across the C++ definition, wrapper configuration, and Python alias, and confirmed that the selected container types differ.

Files Needing Attention: Modules/Core/Mesh/wrapping/test/itkMeshArrayPixelTypeTest.py needs configuration-aware identifier-type selection; Wrapping/Generators/Python/itk/support/types.py is relevant if the alias is corrected centrally.

T-Rex Logs

What T-Rex did

• Validated the posted P1 finding by running the focused Mesh IdentifierType validation script and reviewing the post-change key-type mismatch in the validation log.
• Assessed the contract validation path for the P1 finding, detailing ITK type mappings affecting the Mesh point-data container and noting Windows build limitations, with focused executable source-contract validation confirming the path.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

1. General comment

   Windows builds with 64-bit IDs disabled select the wrong mesh point-data key type

   • Bug
     • itkMeshArrayPixelTypeTest.py line 30 uses itk.IT. On Windows with ITK_USE_64BITS_IDS=OFF, Python maps it to itk.ULL, while the wrapped Mesh[itk.Array.D, 3] uses unsigned long (itk.UL) as its IdentifierType and point-data container key. The resulting VectorContainer[ULL, Array.D] does not match the VectorContainer[UL, Array.D] accepted by mesh.SetPointData(v).
   • Cause
     • Wrapping/Generators/Python/itk/support/types.py sets IT = ULL for every Windows runtime, but the C++ and CMake wrapping choices use ULL only when both Windows and ITK_USE_64BITS_IDS are enabled.
   • Fix
     • Restore configuration-aware container selection in this test (such as the prior ULAD availability check), or make the exported Python itk.IT alias reflect the configured ITK_USE_64BITS_IDS value rather than operating-system identity alone.

   _{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "STYLE: Use `itk.IT` as IdentifierType in..." | Re-trigger Greptile}

[greptile-apps]

Windows identifier types diverge

itk.IT is always exported as itk.ULL on Windows, but a build with ITK_USE_64BITS_IDS=OFF defines itk::IdentifierType as unsigned long and wraps this mesh's point-data container with an itk.UL key. This creates VectorContainer[ULL, Array.D], which does not match the VectorContainer[UL, Array.D] required by mesh.SetPointData(v), so this wrapping test fails on that supported configuration. Restore the configuration-aware UL/ULL container selection, or make itk.IT reflect the configured identifier width.

Artifacts

Focused Mesh IdentifierType selection validation script

• Authored and executed a focused Python source-contract validation for Windows with 64-bit IDs disabled; it asserts the exact C++, wrapping, Python-alias, and changed-test selection facts, with the takeaway that the selected container key types differ.

Focused validation output showing the post-change Mesh key-type mismatch

• Captured output from executing the focused validation script in `/home/user/repo`; it reports `unsigned long` for the mesh and `unsigned long long` for `itk.IT`, with the takeaway that post-change `SetPointData` container types do not match.

_{Ran code and verified through T-Rex}

[N-Dekker]

Hmmm, greptile does not like it. The definition of itk.IT does not take ITK_USE_64BITS_IDS into account. Back to draft!

I don't really understand the original hasattr(itk.VectorContainer, "ULAD"). Is there a more intuitive way to check ITK_USE_64BITS_IDS=OFF in Python?

[dzenanz]

way to check ITK_USE_64BITS_IDS=OFF in Python?

Check size of itk.IT?

[hjmjohnson]

Doing some AI assisted searching.

[hjmjohnson]

Mesh already knows its own IdentifierType, so the test doesn't have to infer it:

MeshType = itk.Mesh[PixelType, Dimension]
mesh = MeshType.New()

IdentifierType = itk.template(mesh.GetPointData())[1][0]
v = itk.VectorContainer[IdentifierType, PixelType].New()

Mesh::PointDataContainer is VectorContainer<PointIdentifier, PixelType>, so [1][0] is IdentifierType as C++ instantiated it — correct on every platform and every ITK_USE_64BITS_IDS setting by construction, because it reads the answer out of the wrapping instead of re-deriving it.

Separately: itk.IT is currently wrong for WIN32 + ITK_USE_64BITS_IDS=OFF, which is what greptile is reacting to. That's pre-existing, not something this PR introduced — but it does block substituting itk.IT here until fixed.

Why itk.IT is wrong, and what hasattr(itk.VectorContainer, "ULAD") was doing

Wrapping/WrapBasicTypes.cmake:221 picks the type with a two-part condition:

if(WIN32 AND ITK_USE_64BITS_IDS)
  set(ITKM_IT ${ITKM_ULL})
else()
  set(ITKM_IT ${ITKM_UL})
endif()

Wrapping/Generators/Python/itk/support/types.py:162-167 mirrors it with only the first half:

IT = UL
if os.name == "nt":
    IT = ULL

So on Windows with ITK_USE_64BITS_IDS=OFF, the wrapping instantiates UL while itk.IT reports ULL.

The original hasattr(itk.VectorContainer, "ULAD") is the same question asked indirectly: UL + AD (Array-Double) is the mangled instantiation name, so its presence proves the actually-wrapped identifier type is UL. It works, but it hard-codes a mangled name that depends on the pixel type, which is why it reads as opaque.

Suggested durable fix for itk.IT (separate PR)

CMake already computes the authoritative answer as ITKM_IT. Plumb that value through rather than re-implementing the condition in Python:

# Wrapping/Generators/Python/itkConfig.template.in.py
ITK_GLOBAL_WRAPPING_BUILD_OPTIONS: dict[str, list[str]] = {
    ...
    "ITKM_IT": "@ITKM_IT@".split(";"),
    "ITKM_ST": "@ITKM_ST@".split(";"),
    "ITKM_OT": "@ITKM_OT@".split(";"),
}

then resolve IT / ST / OT in types.py from those names instead of os.name. itkConfig imports nothing from itk, so there is no circular-import hazard.

The two changes aren't redundant: this one makes itk.IT trustworthy toolkit-wide; the itk.template(...) form above is what a test should do regardless, since it verifies the wrapping rather than trusting a parallel derivation of it.

Verified locally

Against a ITK_WRAP_PYTHON=ON build of current main, with this PR's exact types:

GetPointData template: (itk::VectorContainer, (unsigned long, itkArrayD))
IdentifierType -> unsigned long    # == itk.IT on this (non-Windows) build
VectorContainer[IdentifierType, itk.Array.D].New() -> ok

[N-Dekker]

Thanks @hjmjohnson Instead of fixing IT / ST / OT, I would rather leave them "for legacy only", and then maybe have fixed aliases with more descriptive names (itk.IdentifierType, itk.SizeType, itk.OffsetType?)

Specifically for itk.VectorContainer, I would prefer to allow Python users to leave the index type unspecified, allowing them to simply write itk.VectorContainer[ElementType]. Following pull request #4856, which allows C++ users to write itk::VectorContainer<ElementType> (using itk::SizeValueType as index type, by default).

In practice it is rarely useful (if at all) to have multiple different index types (also known as "element identifier types") for VectorContainer.

What do you think?

Anyway, I think we can abandon this PR without merging, no problem 🤷

[dzenanz]

I like Hans' AI solution. It can be done in another PR. Your last proposal can build on top of that.

[hjmjohnson]

Correction to my comment above: the types.py snippet I quoted is stale — #6767 landed in the meantime and rewrote those lines to

ST = uint64_t
IT = uint64_t
OT = int64_t

The conclusion is unchanged, and arguably sharper: WrapBasicTypes.cmake still selects unsigned long long only when both WIN32 and ITK_USE_64BITS_IDS hold, so on Windows with ITK_USE_64BITS_IDS=OFF the wrapping instantiates unsigned long while itk.IT now unconditionally reports 64-bit.

I've opened #6772 for that, which resolves ST/IT/OT from ITKM_ST/ITKM_IT/ITKM_OT via itkConfig so the alias and the instantiation come from one definition. With it, itk.IT is safe to use here as this PR intends.