Skip to content

COMP: Move ITK Python wrapping to SWIG 4.4.1 #6484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hjmjohnson wants to merge 6 commits into
base: main
Choose a base branch
from
Open

COMP: Move ITK Python wrapping to SWIG 4.4.1 #6484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9e8f0c7
COMP: Support SWIG 4.4.1 multi-phase init in Python wrapping
hjmjohnson Jun 21, 2026
86c13a0
COMP: Vendor SWIG 4.4.1 binaries via FetchContent with version assertion
hjmjohnson Jun 21, 2026
a1612d4
COMP: Use pixi-supplied SWIG 4.4.1 and CastXML for local build-python
hjmjohnson Jun 21, 2026
a05386e
STYLE: Remove unused swig_archive variable
hjmjohnson Jun 21, 2026
94874ca
STYLE: Unify swig_cmake_version into ITK_SWIG_VERSION
hjmjohnson Jun 21, 2026
0a2891b
COMP: Factor swig vendored-download mirror URLs into a helper
hjmjohnson Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions CMake/ITKSetPython3Vars.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,14 @@ else()
set(ITK_WRAP_PYTHON_VERSION "${Python3_VERSION}")

# start section to define package components based on LIMITED_API support and choices
set(_ITK_MINIMUM_SUPPORTED_LIMITED_API_VERSION 3.11)
# Cached so the abi3 floor is a single source of truth shared with the module
# wrapping macros (itk_end_wrap_module.cmake USE_SABI).
set(
_ITK_MINIMUM_SUPPORTED_LIMITED_API_VERSION
3.11
CACHE INTERNAL
"Minimum Python minor version for the abi3 / Limited API floor"
)

# Force ITK_WRAP_PYTHON_VERSION if SKBUILD_SABI_COMPONENT requests it
string(
Expand Down Expand Up @@ -173,7 +180,8 @@ else()
endif()
unset(_missing_required_component)
unset(_python_find_components)
unset(_ITK_MINIMUM_SUPPORTED_LIMITED_API_VERSION)
# _ITK_MINIMUM_SUPPORTED_LIMITED_API_VERSION is cached (INTERNAL) and intentionally
# left set so itk_end_wrap_module.cmake can pin USE_SABI to the same floor.
# end section to define package components based on LIMITED_API support and choices
if(DEFINED _specified_Python3_EXECUTABLE)
set(
Expand Down
51 changes: 45 additions & 6 deletions Wrapping/Generators/Python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,9 @@ macro(
endif()

set(_swig_depend)
if(NOT ITK_USE_SYSTEM_SWIG)
# The ExternalProject SWIG install.
if(NOT ITK_USE_SYSTEM_SWIG AND TARGET swig)
# Build-order against the source-build ExternalProject SWIG target. Prebuilt
# binaries are fetched at configure time (FetchContent) and need no target.
set(_swig_depend swig)
endif()

Expand Down Expand Up @@ -495,10 +496,48 @@ macro(itk_end_wrap_submodule_python group_name)
PyObject * ${base_name}AlreadyImported = PyDict_GetItemString(sysModules, \"itk._${base_name}Python\");
if(${base_name}AlreadyImported == NULL)
{
PyImport_AddModule(\"itk._${base_name}Python\");
PyObject * ${base_name}Module = PyInit__${base_name}Python();
PyDict_SetItemString(sysModules, \"itk._${base_name}Python\", ${base_name}Module);
SWIG_Py_DECREF( ${base_name}Module);
PyObject * ${base_name}Init = PyInit__${base_name}Python();

@hjmjohnson hjmjohnson Jun 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blowekamp, I did not write this code (agentic coding suggestions). In reviewing it, I don't fully understand the subtleties of the Python wrapping at this level. I am hoping that your recent work in SimpleITK makes you a good reviewer for this code.

I'm trying to get ITK in line with SimpleITK.

During the generation of this code, the "agentic code review listing of problems, agentic suggested resolution" dialogs looked reasonable to me, but I was basing the human nudging on facts presented to me that I have not studied deeply in the past.

@blowekamp blowekamp Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this code does not look familiar to me either.

@hjmjohnson hjmjohnson Jun 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dzenanz Can you make a suggestion of who might be a good reviewer of this code?

I originally thought this was going to be a mechanical "update tarball and version numbers" enhancement, but the full implementation of the SABI support required and suggested fortifications.

I'll circle back to it soon if no one else does a careful review. I'll need to brush up on the nuances of what is happening here and research the implementation pattern. Hopefully someone else can review and confirm that the changes are correct. I'll change authorship to who-ever confirms the c++ code changes are correct 🥇 :) .

@dzenanz dzenanz Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matt and Brad are most familiar with wrapping internals.

PyObject * ${base_name}Module = ${base_name}Init;
if (${base_name}Init != NULL && !PyModule_Check(${base_name}Init))
{
// SWIG >= 4.4 uses PEP 489 multi-phase init: PyInit_* returns a
// PyModuleDef that must be instantiated against a spec, not used directly.
${base_name}Module = NULL;
PyObject * ${base_name}Spec = NULL;
PyObject * ${base_name}Machinery = PyImport_ImportModule(\"importlib.machinery\");
if (${base_name}Machinery != NULL)
{
PyObject * ${base_name}SpecType = PyObject_GetAttrString(${base_name}Machinery, \"ModuleSpec\");
if (${base_name}SpecType != NULL)
{
${base_name}Spec = PyObject_CallFunction(${base_name}SpecType, \"sO\", \"itk._${base_name}Python\", Py_None);
SWIG_Py_DECREF(${base_name}SpecType);
}
SWIG_Py_DECREF(${base_name}Machinery);
}
if (${base_name}Spec != NULL)
{
${base_name}Module = PyModule_FromDefAndSpec((PyModuleDef *)${base_name}Init, ${base_name}Spec);
SWIG_Py_DECREF(${base_name}Spec);
if (${base_name}Module != NULL && PyModule_ExecDef(${base_name}Module, (PyModuleDef *)${base_name}Init) != 0)
{
// Exec failed: drop the half-initialized module so it is not cached.
SWIG_Py_DECREF(${base_name}Module);
${base_name}Module = NULL;
}
}
if (${base_name}Module == NULL)
{
// Surface the init failure (prints + clears the exception) so the root
// cause is visible and a later submodule init is not poisoned.
PyErr_WriteUnraisable(${base_name}Init);
}
Comment thread
hjmjohnson marked this conversation as resolved.
}
if (${base_name}Module != NULL)
{
PyDict_SetItemString(sysModules, \"itk._${base_name}Python\", ${base_name}Module);
SWIG_Py_DECREF(${base_name}Module);
}
}
"
)
Expand Down
Loading
Loading