Skip to content

WIP COMP: SWIG 4.4.1 pixi Python wrapping CI (do not merge; revert CI-disable commit)#6481

Closed
hjmjohnson wants to merge 4 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:swig-441-pixi-python-wip
Closed

WIP COMP: SWIG 4.4.1 pixi Python wrapping CI (do not merge; revert CI-disable commit)#6481
hjmjohnson wants to merge 4 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:swig-441-pixi-python-wip

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

WIP / not for merge. Tests the SWIG 4.4.1 Python-wrapping fix in a pixi CI environment. The last commit disables all other CI so only the new ITK.Pixi.Python job runs while iterating — revert that commit to re-enable all CI before this is considered for merge.

Supersedes the temporary 4.3.x baseline in #6480; addresses the import regression in #6479.

What this does
  • Fix (SWIG 4.4.1 multi-phase init): ITK injects C that calls PyInit__<X>Python() and stores the result in sys.modules. SWIG 4.4's PEP 489 multi-phase init returns a raw PyModuleDef, so the proxy fails at import ('moduledef' object has no attribute 'delete_SwigPyIterator'). The injection now detects a returned def at runtime and instantiates it via PyModule_FromDefAndSpec + PyModule_ExecDef; the single-phase (≤4.3) path is unchanged. All APIs are Limited-API-safe (≥3.5).
  • abi3 floor: pinned to a single cached source of truth (_ITK_MINIMUM_SUPPORTED_LIMITED_API_VERSION = 3.11). 3.11+ build abi3/Limited-API; the existing auto-detect keeps 3.10 (EOL pre-v6) on the non-Limited-API path.
  • pixi env: swig (>=4.4.1), castxml, ccache added to the python feature; configure-python passes ITK_USE_SYSTEM_SWIG/CASTXML.
  • New CI: .github/workflows/pixi-python.yml — builds Python wrapping with system SWIG 4.4.1 + CastXML on ubuntu/windows/macos, runs an import + filter + std::vector/SwigPyIterator smoke test and scoped Python regression tests.
Local validation (macOS arm64)

Builds and imports on Python 3.11.15 and 3.13.9 (same abi3 build); 10/10 scoped Python ctests pass; std::vector/SwigPyIterator path verified.

WIP-disabled CI (revert the last commit to restore)

GHA arm, doxygen, perf-benchmark, pixi (cxx), pre-commit set to workflow_dispatch; Azure Linux, LinuxPython, MacOSPython, WindowsPython set pr: none (Batch was already pr: none). Path-gated / pull_request_target housekeeping workflows are untouched (they do not fire on this PR's changes).

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Jun 20, 2026
@hjmjohnson hjmjohnson force-pushed the swig-441-pixi-python-wip branch from 19acf73 to f90cc56 Compare June 20, 2026 18:47

@dzenanz dzenanz left a comment

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.

Does multi-phase import improve instantiation speed?

@hjmjohnson

Copy link
Copy Markdown
Member Author

Good question — short answer: no, and it isn't meant to.

Multi-phase init isn't something we're opting into for performance; SWIG 4.4 makes PEP 489 multi-phase init the only module-init mode (PyInit_* now returns a PyModuleDef rather than a populated module). This change just adapts ITK's multi-module-per-.so loader to that so wrapping stays importable under 4.4 — it's a correctness fix, not an optimization.

On speed it's effectively neutral. The costly part of loading a wrapped module is SWIG's type-table registration, which now runs in the exec phase — the same work the old single-phase PyInit_* did inline. Fix A only adds, per submodule, a ModuleSpec allocation plus an importlib.machinery lookup (cached after the first call) — sub-millisecond against the ~600 ms first-module type registration I measured locally. Runtime object/filter instantiation is untouched; this only affects module import.

I don't have a clean before/after since the pre-fix build doesn't import at all under SWIG 4.4.1, but I can add a timing line to the new ITK.Pixi.Python job if a tracked import-time number would be useful.

@dzenanz

dzenanz commented Jun 20, 2026

Copy link
Copy Markdown
Member

Thanks for explanation.

Filter instantiations are slow, running in the seconds or dozens of seconds. From my experience, that is the biggest pain point of ITK Python.

@hjmjohnson hjmjohnson force-pushed the swig-441-pixi-python-wip branch 3 times, most recently from 34a38fa to 22bd48a Compare June 21, 2026 02:48
ITK packs several SWIG modules per shared library and injects C that calls
PyInit__<X>Python() and stores the return directly in sys.modules. SWIG 4.4
(PEP 489 multi-phase init) makes PyInit_* return a raw PyModuleDef rather than
a populated module, so the proxy fails at import with 'moduledef' object has no
attribute 'delete_SwigPyIterator'. Detect a returned def at runtime and
instantiate it via PyModule_FromDefAndSpec + PyModule_ExecDef; the single-phase
(<=4.3) path is unchanged. All APIs used are Limited-API-safe (>=3.5).

Pin the abi3 floor to a single cached source of truth
(_ITK_MINIMUM_SUPPORTED_LIMITED_API_VERSION = 3.11) shared by the Limited-API
auto-detect and the USE_SABI setting, so 3.11+ builds are abi3 and load on 3.11.

Assisted-by: Claude Code — root-cause analysis and local 3.11/3.13 validation
…tXML

Add swig (>=4.4.1), castxml, and ccache to the python pixi feature and pass
ITK_USE_SYSTEM_SWIG/ITK_USE_SYSTEM_CASTXML in configure-python, so wrapping uses
the conda-forge toolchain instead of the internal download/build.
New workflow builds the Python wrapping with system SWIG 4.4.1 + CastXML on
ubuntu/windows/macos, runs an import + filter + std::vector smoke test, and a
scoped set of Python regression tests.
TEMPORARY scaffolding to iterate on the SWIG 4.4.1 Python wrapping:
- Co-opts the reliably-triggering ITK.Pixi workflow (pixi.yml) to run the
  Python wrapping via configure-python-ci / build-python-ci / test-python-ci
  (system SWIG/CastXML + ccache in the python env) instead of the cxx env.
- Neuters the PR triggers of the remaining build/check CI (arm, doxygen,
  perf-benchmark, pre-commit GHA set to workflow_dispatch; Azure
  Linux/LinuxPython/MacOSPython/WindowsPython set pr: none).
REVERT THIS COMMIT before merge to restore the normal cxx ITK.Pixi CI and
re-enable all other CI.
@hjmjohnson hjmjohnson force-pushed the swig-441-pixi-python-wip branch from 22bd48a to f3a0ca1 Compare June 21, 2026 02:52
@hjmjohnson

Copy link
Copy Markdown
Member Author

Thanks for explanation.

Filter instantiations are slow, running in the seconds or dozens of seconds. From my experience, that is the biggest pain point of ITK Python.

That is outside of the scope of this work. Should be investigated separately :).

@hjmjohnson

Copy link
Copy Markdown
Member Author

PS: ITKv6 is likley not going to get a release until very close to EOL for python 3.10. Removing Python 3.10 support cleans up a lot of SABI NO_SABI code. and simplifies the number of environments and python packages we will need to support. I'm working on making a PR to implement that for feedback.

@hjmjohnson

Copy link
Copy Markdown
Member Author

Superseded by #6484 — the final, WIP-free SWIG 4.4.1 PR with the multi-phase-init fix, vendored 4.4.1 binaries via FetchContent, and standard GHA + Azure CI. @dzenanz your multi-phase-init question is addressed there (it's a correctness adaptation to SWIG 4.4's PEP 489 init, performance-neutral). Closing in favor of #6484.

@hjmjohnson hjmjohnson closed this Jun 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Python wrapping Python bindings for a class type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants