Skip to content

ENH: Bump SWIG to 2024-03-26-master #4541

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

Jump to bottom
Merged
thewtex merged 6 commits into from
Apr 1, 2024
Merged

ENH: Bump SWIG to 2024-03-26-master #4541

thewtex merged 6 commits into from
Apr 1, 2024

Conversation

@thewtex
Copy link
Member

@thewtex thewtex commented Mar 27, 2024 •
edited by dzenanz
Loading

  • Supports more modern C++
  • More Pythonic interfaces, e.g. Python Iterator Protocol
  • Python Limited ABI

There were duplicate definition errors due to double inclusion of

%template(vectoritkImageXX, std::vector< itkImageXX >)

via itkImage_ext.i in PCAShapeSignedDistanceFunction. Remove
itkImage_ext.i from the latter but still provide the required typemaps
by adding DECL_PYTHON_STD_VEC_RAW_TO_SMARTPTR_TYPEMAP in
itkPCAShapeSignedDistanceFunction.wrap.

Remove -py3 because it is no longer supported (the default).

Remove "ios_base*" config templates because they are no longer emitted.

Bump PCRE. SWIG and PCRE are now built via CMake instead of autotools.

Closes #4536
Closes #3364
Closes #3955

@thewtex
ENH: Bump SWIG to 2024-03-26-master
d6c69f5 
- Supports more modern C++
- More Pythonic interfaces, e.g. Python Iterator Protocol
- Python Limited ABI

There were duplicate definition errors due to double inclusion of

  %template(vectoritkImageXX, std::vector< itkImageXX >)

via itkImage_ext.i in PCAShapeSignedDistanceFunction. Remove
itkImage_ext.i from the latter but still provide the required typemaps
by adding DECL_PYTHON_STD_VEC_RAW_TO_SMARTPTR_TYPEMAP in
itkPCAShapeSignedDistanceFunction.wrap.

Remove -py3 because it is no longer supported (the default).

Remove "ios_base*" config templates because they are no longer emitted.

Bump PCRE. SWIG and PCRE are now built via CMake instead of autotools.
@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Python wrapping Python bindings for a class area:Segmentation Issues affecting the Segmentation module labels Mar 27, 2024
@thewtex
Copy link
Member Author

thewtex commented Mar 27, 2024

@dzenanz note how to support std::vector< ImageType > on class APIs.

@thewtex
Copy link
Member Author

thewtex commented Mar 27, 2024 •
edited
Loading

Closes #4536
Closes #3364

@N-Dekker
Copy link
Contributor

N-Dekker commented Mar 27, 2024

Wow, that's a big step forward! Thanks Matt!

Still wondering, is there a specific reason to take the revision of 2024-03-26-master, rather than the last official release, https://github.com/swig/swig/releases/tag/v4.2.1 ?

@thewtex
Copy link
Member Author

thewtex commented Mar 27, 2024

Still wondering, is there a specific reason to take the revision of 2024-03-26-master, rather than the last official release, https://github.com/swig/swig/releases/tag/v4.2.1 ?

Yes, we want this patch: swig/swig@21aa6e8

@dzenanz
Copy link
Member

dzenanz commented Mar 28, 2024

Does this touch #3955?

@dzenanz
Copy link
Member

dzenanz commented Mar 28, 2024

MacOS Python build error:

CMake Error at /usr/local/Cellar/cmake/3.28.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find BISON: Found unsuitable version "2.3", but required is at
  least "3.5" (found /usr/bin/bison)
Call Stack (most recent call first):
  /usr/local/Cellar/cmake/3.28.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:598 (_FPHSA_FAILURE_MESSAGE)
  /usr/local/Cellar/cmake/3.28.1/share/cmake/Modules/FindBISON.cmake:306 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  CMakeLists.txt:91 (find_package)

dzenanz
dzenanz approved these changes Mar 28, 2024
View reviewed changes
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Code looks good. Maybe we need a newer MacOS virtual machine image?

Modules/Segmentation/SignedDistanceFunction/wrapping/itkPCAShapeSignedDistanceFunction.wrap
# `double` because of OptimizerParameters is hardcoded as templated
# over `double` in parent class `ShapeSignedDistanceFunction`.
foreach(t ${types})
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_PYTHON_STD_VEC_RAW_TO_SMARTPTR_TYPEMAP(itkImage${ITKM_${t}}${d}, itkImage${ITKM_${t}}${d}_Pointer)\n")
Copy link
Member

@dzenanz dzenanz Mar 28, 2024

Choose a reason for hiding this comment

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

Is this expected to work in remote modules? Should I replace the old way with this style?

Copy link
Member Author

@thewtex thewtex Mar 28, 2024

Choose a reason for hiding this comment

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

Yes

@dzenanz
Copy link
Member

dzenanz commented Mar 28, 2024

I assume we need to bump the version here:

ITK/Testing/ContinuousIntegration/AzurePipelinesMacOS.yml

Line 34 in fad5849

vmImage: macos-11

@thewtex
Copy link
Member Author

thewtex commented Mar 28, 2024

Does this touch #3955?

I will add macOS binaries.

@thewtex thewtex marked this pull request as draft March 28, 2024 12:52
@thewtex
ENH: Add macOS ARM SWIG binary
2df824b 
Built with MACOSX_DEPLOYMENT_TARGET set to 10.11.
@thewtex
Copy link
Member Author

thewtex commented Mar 28, 2024

I assume we need to bump the version here:

No, we want to keep testing against the older version, and bumping would not help with the build.

thewtex added 4 commits March 28, 2024 15:07
@thewtex
ENH: Add macOS AMD64 SWIG binary
b507643 
Built with MACOSX_DEPLOYMENT_TARGET=10.11.
@thewtex
ENH: Add Linux aarch64 SWIG binary
82d8165 
Built with quay.io/pypa/manylinux_2_28_aarch64:2024-03-25-9206bd9.
@thewtex
STYLE: Simplify SWIG CMake variables
c12eaef 
All the binary versions are required minimum versions are now the same
per-platform.
@thewtex
ENH: Add Python Limited API support
0c5e523 
Builds for Python 3.11 will work on 3.11 or greater CPython.

3.11 is required for the Buffer Protocol API's used in our NumPy bridge..
@thewtex thewtex marked this pull request as ready for review March 29, 2024 15:17
@github-actions github-actions bot added the area:Bridge Issues affecting the Bridge module label Mar 29, 2024
@thewtex
Copy link
Member Author

thewtex commented Mar 29, 2024

/azp run ITK.Windows

@thewtex thewtex merged commit a648bd5 into InsightSoftwareConsortium:master Apr 1, 2024
thewtex added a commit to thewtex/ITKPythonPackage that referenced this pull request Apr 1, 2024
@thewtex
ENH: Bump ITK for Limited API support
ca8c3b2 
Includes: InsightSoftwareConsortium/ITK#4541
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dzenanz dzenanz dzenanz approved these changes

@N-Dekker N-Dekker Awaiting requested review from N-Dekker

@hjmjohnson hjmjohnson Awaiting requested review from hjmjohnson

@jhlegarreta jhlegarreta Awaiting requested review from jhlegarreta

Assignees

No one assigned

Labels

area:Bridge Issues affecting the Bridge module area:Python wrapping Python bindings for a class area:Segmentation Issues affecting the Segmentation module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Upgrade SWIG from version 4.0.2 to version 4.1.0 or higher, please! Create SWIG binaries for macOS PyEval_InitThreads is deprecated in Python 3.9

3 participants

@thewtex @N-Dekker @dzenanz