slicersalt / ITKShape

ITK-Based Shape Analysis Filters (e.g Procrustes Alignment for Meshes)
Apache License 2.0
4 stars 6 forks source link

BUG: Filter not accepting mesh input for Python wrapping #10

Closed tbirdso closed 3 years ago

tbirdso commented 3 years ago

Overview

MeshProcrustesAlignFilter builds and is wrapped successfully for template arguments of type itk::Mesh<double, 3>. However, attempting to set input meshes in Python following setting number of inputs returns an error:

>>> filter.SetInput(0,m1) ... TypeError: in method 'itkMeshProcrustesAlignFilterM3M3_SetInput', argument 3 of type 'itkMeshD3_Pointer'

At face value it seems that there is a type mismatch between the expected argument of type itkMeshD3_Pointer and the given value of type itkMeshD3. However, per Discourse discussion Python is not expected to handle pointers directly.

Steps to Reproduce

  1. Read in valid ITK::mesh<double,3> in Python and validate type:

    m1 = itk.meshread('C:/users/tom.birdsong/source/repos/hasi/test/input/901-L-mesh.vtk', itk.D)

type(m1) <class 'itk.itkMeshBasePython.itkMeshD3'>

  1. Instantiate filter and validate type: filter = itk.MeshProcrustesAlignFilter[itk.Mesh[itk.D,3], itk.Mesh[itk.D,3]].New()

type(filter) <class 'itk.itkMeshProcrustesAlignFilterPython.itkMeshProcrustesAlignFilterM3M3'>

  1. Set number of filter inputs (for example, trying to find the mean of two meshes) filter.SetNumberOfInputs(2)
  2. Attempt to set first mesh input filter.SetInput(0,m1)

Expected behavior

Setting inputs up to the allocated number in Step 3 succeeds without error.

Observed behavior

Traceback (most recent call last): File "", line 1, in TypeError: in method 'itkMeshProcrustesAlignFilterM3M3_SetInput', argument 3 of type 'itkMeshD3_Pointer'

tbirdso commented 3 years ago

@thewtex @dzenanz Could you add insight as to how pointer arguments might be causing issues in Python here? Is it likely an issue with the module wrapping, or a symptom of a different root cause?

dzenanz commented 3 years ago

In 2018, mesh support in Python was deficient (https://github.com/InsightSoftwareConsortium/ITKExamples/pull/66). Maybe not everything has been updated properly in ITK? If you try updating the example in https://github.com/InsightSoftwareConsortium/ITKExamples/pull/66 maybe you could get a better grip of mesh support. Trying it out on a simple, stand-alone example should be helpful for this custom filter.

tbirdso commented 3 years ago

I'll check it out, thanks!

thewtex commented 3 years ago

@tbirdso The SetInput method currently has a Mesh SmartPointer as its argument:

https://github.com/slicersalt/ITKMesh3DProcrustesAlignFilter/blob/091fb7c4998fd9182108adbfe0275fa764dca453/include/itkMeshProcrustesAlignFilter.h#L104

This needs to be a raw pointer instead of a SmartPointer. Passing a smart pointer causes unnecessary smart pointer construction / destruction, and wrapping and wrapping ownership rules are set up for raw pointers.

tbirdso commented 3 years ago

Thanks @thewtex. I wasn't understanding that Pointer = SmartPointer< Self >, but now I see that in the itk::Object documentation as well.

I found Cpp guidelines to help myself better understand the issue with this design pattern: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-smart