ITK icon indicating copy to clipboard operation
ITK copied to clipboard

ITK filter methods that want a sequence of ints get confused when passed a sequence of np.int64s

Open HastingsGreer opened this issue 2 years ago • 3 comments

Description

When you index into a numpy array and think you got an integer, you actually get a numpy.int64. python ints and numpy.int64 are almost indistinguishable, but If you pass a sequence of numpy.int64 s to an ITK method that expects a sequence of integers, it throws an inscrutable error. This is unpleasant to debug.

Steps to Reproduce

import itk
import numpy as np

crop_amounts = np.array([1, 2, 3])
crop_amounts = [i for i in crop_amounts]

print(crop_amounts)
# [1, 2, 3]

filter = itk.CropImageFilter[itk.Image[itk.F, 3], itk.Image[itk.F, 3]].New()
filter.SetLowerBoundaryCropSize(crop_amounts)

Expected behavior

sets the lower boundary

Actual behavior

Traceback (most recent call last):
  File "<stdin>", line 8, in <module>
ValueError: Expecting a sequence of int (or long)

Reproducibility

100%

Versions

any

Environment

any

Additional Information

HastingsGreer avatar May 05 '23 01:05 HastingsGreer

Looking at this currently.

arhowe00 avatar Jan 23 '25 15:01 arhowe00

Hey @HastingsGreer thanks for opening this issue. To summarize your issue so far...

numpy.int64 cannot be implicitly cast to SizeType. In your specific case, that is Modules/Filtering/ImageGrid/include/itkCropImageFilter.h, where SizeType is derived from Modules/Core/Common/include/itkSize.h.

@dzenanz suggested augmenting the wrappings so that numpy.int64 can be cast to SizeType. I think this would mean adding a SWIG typemap in Wrapping/Generators/Python/PyBase/pyBase.i. However, since SizeType ends up as itkSize1, itkSize2, ..., I was thinking of doing the following in pyBase.i:

  1. Make a generic %define DECL_NUMPY_VEC_TYPEMAP(swig_name, dim) that will define %typemap(in)s to each of the supported itk::Size types, given the numpy array's dimensions.
  2. Add a macro to Wrapping/Generators/Python/CMakeLists.txt which grabs the dim from the template and passes it to DECL_NUMPY_VEC_TYPEMAP. The following example is similar to existing macros:
macro(ADD_NUMPY_VEC_TYPEMAP swig_name template_param)
  if(NOT isTemplate)
    string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_NUMPY_VEC_TYPEMAP(${swig_name}, ${template_param})\n")
  endif()
endmacro()
  1. We add the following line to Wrapping/TypedefMacros.cmake, which will call the macro for the given type we want to convert from numpy.int64. This will create the typemaps in build/tree/Wrapping/Typedefs/python/itkSize_ext.i:
if("${cpp_name}" STREQUAL "itk::Size")
  add_numpy_vec_typemap("${swig_name}" "${template_params}")
endif()

I'm having trouble with this approach. First, to cast numpy.int64 to itk::Size type, according to the numpy docs, it seems we will need to add the file numpy.i, because it exposes the SWIG interface for numpy types. Furthermore, would require access to numpy's c layer in github.com/numpy/numpy/_core/include/arrayobject.h, and numpy is not currently vendored in Modules/External.

Do I need to vendor numpy to solve this issue? I'm wondering whether people with more experience in SWIG can offer insights as to whether mine is a reasonable solution, or if I should try to address things differently. I've seen @thewtex edit pyBase.i recently, perhaps he has some advice?

arhowe00 avatar Jan 23 '25 21:01 arhowe00

Hi @arhowe00 ,

No, a build dependency on numpy.i should not be brought in. This would add considerable more complexity to the build process, undesirable coupling with NumPy versions, and bloat to the bindings that were explicitly avoided. We have good NumPy support without needing it.

For an alternative approach, see, for instance, how itk.Matrix is handled in pyBase.i.

thewtex avatar Jan 27 '25 17:01 thewtex