InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

Future consideration: are the ITKVtkGlue Python typemaps still needed, given the numpy bridge?

Open
#6,847 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

### Proposal

Record, for consideration in a **future** ITK release, whether the *Python wrapping* of `ITKVtkGlue`'s VTK-pointer typemaps is still needed, given that `itk.vtk_image_from_image` / `itk.image_from_vtk_image` in `itk/support/extras.py` appear to cover the same ground with less coupling.

**This is not a removal request, and nothing should change now.** The C++ side of `ITKVtkGlue` is in active downstream use and is out of scope entirely. The question is narrower: whether the SWIG typemaps that exchange raw `vtkImageData*` / `vtkPolyData*` pointers with Python earn their maintenance cost.

*(Edited: an earlier version of this issue claimed the numpy bridge had an object-lifetime hazard that had to be fixed first. Testing shows it does not — see the correction comment below. The section is retained, corrected, because the reference chain is worth documenting.)*

### Why raise it now

PR #6715 made the typemaps `abi3`-compatible. Doing so required hand-parsing VTK's undocumented `__this__` / `Addr=0x…` encoding and dropping `VTK::WrappingPythonCore` from the link interface, because that library pins an extension to one `libpython`. The work is sound, but it is ongoing maintenance against VTK internals that carry no compatibility guarantee.

That is a reasonable moment to ask whether the coupling is necessary at all.

Measurements comparing the two bridges

Tested on macOS arm64 against a purpose-built VTK 9.6.2, using the PR #6715 branch.

| Property | numpy bridge | typemap bridge |
|---|---|---|
| Spacing, origin | preserved | preserved |
| Direction matrix (non-identity, VTK ≥ 9) | preserved | preserved |
| Buffer sharing (ITK edit visible through VTK) | yes | yes |
| Vector / CovariantVector / tensor pixel types | explicit handling in `extras.py` | via the ITK filter |
| Requires ITK binaries compiled against VTK | **no** | **yes** |
| Participates in the ITK pipeline | no | yes |

Cross-boundary pipeline behaviour was also checked, since that is the capability one would expect the filter to add:

```python
itk_smooth.SetSigma(2.0)
vtk_smooth.Update() # only the VTK side updated
```

The VTK filter returns the **stale** value. Propagation requires an explicit `bridge.Update()` first. The two pipeline graphs are joined by a manual update either way, so the filter is a pipeline *object* but not a pipeline *connection* — it does not fuse the two executives.

With the numpy helper the equivalent code needs `itk_smooth.Update()` and a re-conversion. Different ergonomics, same manual synchronisation, no capability that is reachable one way and not the other.

Who actually uses which path

Surveyed across a local multi-project build of ITK's downstream consumers.

**C++ users of the `ITKVtkGlue` filters — unaffected by any Python question:**

- Slicer — `Libs/vtkITK/vtkITKImageSequenceReader.cxx`, `vtkITKGrowCut.cxx`, `vtkITKLabelShapeStatistics.cxx`
- BRAINSTools — `ConvertBetweenFileFormats/castconverthelpers.h`, `BRAINSConstellationDetector/gui/…`
- ANTs — `Examples/antsSurf.cxx`, `antsVol.cxx`, `Temporary/itkFEM*Map.h`
- LesionSizingToolkit

**Python users of the numpy helpers:**

- Slicer — `Base/Python/slicer/util.py:3060,3126` calls `itk.image_from_vtk_image` / `itk.vtk_image_from_image`

**Python users of the typemaps:**

- Four ITKSphinxExamples: `ConvertAnitkImageTovtkImageData`, `ConvertvtkImageDataToAnitkImage`, `ConvertRGBvtkImageDataToAnitkImage`, `ConvertAnRGBitkImageTovtkImageData`
- Inside ITK, one test: `Wrapping/Generators/Python/Tests/verifyGetOutputAPIConsistency.py`

So the largest Python consumer already uses the numpy path, and the typemap path's Python users are primarily documentation examples.

### Object lifetime in the numpy bridge — checked, and safe

`vtk_image_from_image` builds a `vtkImageData` around `itk.array_view_from_image(...)`, which is a view rather than a copy. That looks like a dangling-buffer risk when reading the function alone, but both retaining links are established outside it:

```
vtkImageData -> vtkTypeFloat32Array -> (._numpy_reference) -> numpy array -> (.base) -> itk.Image
```

`numpy_to_vtk` stashes the source array on the VTK object; ITK's numpy support sets the array's `base` to the image. A `vtkImageData` therefore keeps its source `itk.Image` alive by itself. Verified against VTK 9.6.2 with a control confirming an unconverted image does die when unreferenced, and under heap churn after scope exit.

Caveat: `_numpy_reference` is VTK implementation behaviour, not a documented API guarantee, and I checked only 9.6.2 while ITK's minimum is 9.1. A regression test asserting that a `vtkImageData` outlives its source image's direct references would make the property explicit — worth adding independently of this proposal.

### Suggested disposition

- **Now:** no change. Merge and maintain #6715 as-is.
- **Next:** add coverage asserting that a `vtkImageData` outliving its source `itk.Image` stays valid, so the reference chain is guaranteed rather than incidental.
- **A future release:** revisit whether the Python typemaps are still worth their coupling to VTK internals, with the SphinxExamples migrated first if so. Any deprecation would need the usual `ITK_LEGACY_REMOVE` cycle.
- **Never in scope here:** the C++ `ITKVtkGlue` classes, which have real downstream users.

I am happy to be wrong about this — if there is a use case the numpy path cannot serve, this issue is the place to record it, and it should then be closed.

Contributor guide

Open the contributing guide

Research direction

Start by reading Wrapping/Generators/Python/itk/support/extras.py, verifyGetOutputAPIConsistency.py, and the four ITKSphinxExamples, then compare their use with PR #6715. Done means recording whether the typemaps provide a unique Python capability and, if removal remains justified, identifying the migration and deprecation scope; this issue requests no immediate code change.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, numpy, python
Domain
developer-experience, documentation, testing
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.