STYLE: 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

The mesh array pixel-type wrapping test now uses ITK’s configured identifier alias for its point-data container, preserving compatibility with the mesh wrapper’s identifier type across supported identifier-width configurations.

Confidence Score: 5/5

No blocking failure remains.

The configuration-level validation confirmed that the point-data container uses the same configured identifier token required by the wrapped mesh.

T-Rex Logs

What T-Rex did

• Ran the identifier-type-32-64-validation.py script to verify that the ITKT_IT identifier token is used in the mesh wrapping declaration.
• The validation reported RESULT: PASS and confirmed that Modules/Core/Mesh/wrapping/test/itkMeshArrayPixelTypeTest.py:30 creates itk.VectorContainer[itk.IT, PixelType] and that line 41 passes v to mesh.SetPointData(v).
• An attempt to invoke a native ITK Python binding was made, but the ITK Python module is not present in the environment, so the invocation could not run.
• The second contract-validation shows the same line 30 usage and reports a PASS, with no source changes required.

_{Ran code and verified through T-Rex}

_{Reviews (2): 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.