ENH: Add "itkPointSetTest.py" unittest, testing `SetPointsByCoordinates`
Specifically tested SetPointsByCoordinates with a NumPy array as input argument, as was suggested by @PranjalSahu.
- Follow-up to pull request https://github.com/InsightSoftwareConsortium/ITK/pull/4850 commit f43b1b8977fa05d169c14b4c6f44ce35992a3959
ENH: Add
PointSet::SetPointsByCoordinates(coordinates)
/azp.run ITK.macOS.Arm64
/azp run ITK.macOS.Arm64
/azp run ITK.macOS.Python
/azp run ITK.macOS
/azp run ITK.macOS.Arm64
@dzenanz FYI "ITK.macOS.Arm64"does not listen to /azp run, because it's not using Azure Pipelines!
I just pressed "Re-run jobs" for this GitHub Action, at https://github.com/InsightSoftwareConsortium/ITK/actions/runs/11019940074/job/30603577840?pr=4860
Just checked, the test has actually run, and passed successfully: https://open.cdash.org/tests/1733705147
Test: itkPointSetTest (Passed)
Command Line:
/home/vsts/work/1/s-build/Wrapping/Generators/Python/itk/itkTestDriver "--add-before-env" "PYTHONPATH" "/home/vsts/work/1/s-build/Wrapping/Generators/Python" "--add-before-env" "PYTHONPATH" "/home/vsts/work/1/s-build/Wrapping/Generators/Python/itk" "--add-before-libpath" "/home/vsts/work/1/s-build/Wrapping/Generators/Python/itk" "/opt/hostedtoolcache/Python/3.9.20/x64/bin/python3" "/home/vsts/work/1/s/Modules/Core/Common/wrapping/test/itkPointSetTest.py"
👍
My very first ITK Python unittest 😇
Instead of Unsigned Char, can we instead use float ? Because that will be the most used datatype.
We should also replace all the older tests using SetPoints with the new method to ensure everything else is working.
We should also replace all the older tests using SetPoints with the new method to ensure everything else is working.
@PranjalSahu Please feel free to make another pull request to do so. (Otherwise I can also give it a try, once this pull request is merged.)
- While the merged pull request #4896 already extended the existing serialization tests in Python to use
SetPointsByCoordinates, I still think it would be useful to specifically testSetPointsByCoordinatesin isolation, as I'm proposing with this unit test.
@N-Dekker Pull requests do not seem to be available for your branch, but a few improvements are added to your branch here:
https://github.com/thewtex/ITK/tree/itkPointSetTest-Python-unittest
which adds support for passing a n_points x dim to SetPointsByCoordinates (which is what I think people expect from a Python interface). It also adds a GetPointsByCoordinates that returns the same. And verifies with numpy.testing.assert_allclose.
https://github.com/thewtex/ITK/tree/itkPointSetTest-Python-unittest
which adds support for passing a n_points x dim to
SetPointsByCoordinates(which is what I think people expect from a Python interface). It also adds aGetPointsByCoordinatesthat returns the same. And verifies withnumpy.testing.assert_allclose.
Thanks Matt! I think it's very nice to add support for 2D (number of points x dimension) numpy arrays. The approach of using %rename and %extend swig_name is new to me, cool!
I'm glad to see your proposal still preserves the original support of flat arrays of coordinates as was added by PR #4850 and included with ITK v6.0a01. (I think that was an essential feature, in order to provide an alternative to the problematical SetPoints overload.) So your proposal looks like an enhancement on top of PR #4850 and #4860 (this one), right? As such, I would rather see this one (PR #4860) being merged, so that we can then review your proposal as a follow-up PR. Would that be OK to you?
:+1: yes, @N-Dekker sounds like a good plan.