Skip to content

Function_Warp improvements and unit tests#77

Open
Tiomat85 wants to merge 4 commits intonion-software:masterfrom
Tiomat85:function_warp
Open

Function_Warp improvements and unit tests#77
Tiomat85 wants to merge 4 commits intonion-software:masterfrom
Tiomat85:function_warp

Conversation

@Tiomat85
Copy link

This adds unit tests for function_warp testing various data types and shapes, and validates shape and quick sanity check of data. Additionally extended the function_warp functionality itself to be able to handle data of higher dimensionality than the provided warp map, with it broadcasting the map over the higher dimensions. So as an example if the data provided was of shape (8,4,32,32) and the warp map was (32,32) it broadcasts it applying the warp map to each data dimension correctly for each of the sequence/scan dimensions.

Tiomat85 and others added 3 commits January 15, 2026 16:03
This adds unit tests for function_warp testing various data types and shapes, and validates shape and quick sanity check of data.
Additionally extended the function_warp functionality itself to be able to handle data of higher dimensionality than the provided warp map, with it broadcasting the map over the higher dimensions.
So as an example if the data provided was of shape (8,4,32,32) and the warp map was (32,32) it broadcasts it applying the warp map to each data dimension correctly for each of the sequence/scan dimensions.
Added is_channel_data to specify whether data has the last dimension as channel (rgb/rgba) and so to correctly validate the warped shape.

Missed return type, Adding typing to validate_warp
Copy link
Collaborator

@cmeyer cmeyer left a comment

Choose a reason for hiding this comment

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

Overall code looks good other than the few nitpicks I put in comments.

@cmeyer
Copy link
Collaborator

cmeyer commented Jan 16, 2026

Btw - I can make all these changes when I merge it, if you want.

Removed test-test code.
Adds function description comment.
Utilise better properties to identify if data is channelled.
def function_warp(data_and_metadata_in: _DataAndMetadataLike, coordinates_in: typing.Sequence[
_DataAndMetadataLike], order: int = 1) -> DataAndMetadata.DataAndMetadata:
def function_warp(data_and_metadata_in: _DataAndMetadataLike, coordinates_in: typing.Sequence[_DataAndMetadataLike], order: int = 1) -> DataAndMetadata.DataAndMetadata:
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll fix this during the merge, but this is the wrong format for a Python function comment.

https://peps.python.org/pep-0257/#multi-line-docstrings

@Tiomat85 Tiomat85 requested a review from cmeyer March 2, 2026 09:25
Copy link
Collaborator

@cmeyer cmeyer left a comment

Choose a reason for hiding this comment

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

The new tests don't actually test anything, mainly due to _validate_warp not using the src variable. This is easy to see by changing the two map_coordinates lines in Core.py to pass # .... The new tests still pass. The pre-existing tests fail, of course.

Also, I'd like to avoid unnecessary abbreviations like idx in place of the more descriptive index. If you want a starting point that fixes comments and many (but not all) variable names, here is a diff that you can apply before fixing the tests.

As a reminder the top comment should describe what the function does, not how it does it. The top comment is used by callers, who typically don't care how it is implemented. So I moved the map_coordinates comments next to the function calls where they will be seen by anyone working on the implementation.

diff --git a/nion/data/Core.py b/nion/data/Core.py
index 015f046..6763dc8 100755
--- a/nion/data/Core.py
+++ b/nion/data/Core.py
@@ -1801,16 +1801,13 @@ def function_resample_2d(data_and_metadata_in: _DataAndMetadataLike, shape: Data
 
 
 def function_warp(data_and_metadata_in: _DataAndMetadataLike, coordinates_in: typing.Sequence[_DataAndMetadataLike], order: int = 1) -> DataAndMetadata.DataAndMetadata:
+    """Warp or unwarp input data using an N-dimensional warp map.
+
+    The warp map is applied along N axes and broadcast over any additional
+    dimensions in the input, allowing a single warp map to be used for
+    higher-dimensional data (e.g., image sequences). For multichannel data
+    such as RGB/RGBA, the warp is applied uniformly to all channels.
     """
-        Warp or unwarp input data using an N-dimensional warp map.
-
-        The warp map is applied along N axes and broadcast over any additional
-        dimensions in the input, allowing a single warp map to be used for
-        higher-dimensional data (e.g., image sequences). For channelled data
-        such as RGB/RGBA, the warp is applied uniformly to all channels.
-        
-        scipy map_coordinates does not broadcast by default, so need to loop
-        """
     data_and_metadata = DataAndMetadata.promote_ndarray(data_and_metadata_in)
     coordinates = [DataAndMetadata.promote_ndarray(c) for c in coordinates_in]
     coords = numpy.stack([c.data.astype(float) for c in coordinates], axis=0)
@@ -1824,12 +1821,11 @@ def function_warp(data_and_metadata_in: _DataAndMetadataLike, coordinates_in: ty
         channels = 3 if data_and_metadata.is_data_rgb else 4
         output = numpy.zeros(tuple(output_shape) + (channels,), numpy.uint8)
 
-        for idx in numpy.ndindex(leading_shape):
-            for chan in range(channels):
-                output[idx + (..., chan)] = scipy.ndimage.map_coordinates(
-                    data[idx + (..., chan)],
-                    coords,
-                    order=order)
+        # scipy map_coordinates does not broadcast by default, so need to loop
+        for index in numpy.ndindex(leading_shape):
+            # loop over each rgb/a channel.
+            for channel in range(channels):
+                output[index + (..., channel)] = scipy.ndimage.map_coordinates(data[index + (..., channel)], coords, order=order)
 
         return DataAndMetadata.new_data_and_metadata(data=output,
                                                      dimensional_calibrations=data_and_metadata.dimensional_calibrations,
@@ -1839,8 +1835,9 @@ def function_warp(data_and_metadata_in: _DataAndMetadataLike, coordinates_in: ty
         output_shape = leading_shape + coords.shape[1:]
         output = numpy.zeros(output_shape, dtype=data.dtype)
 
-        for idx in numpy.ndindex(leading_shape):
-            output[idx] = scipy.ndimage.map_coordinates(data[idx], coords, order=order)
+        # scipy map_coordinates does not broadcast by default, so need to loop
+        for index in numpy.ndindex(leading_shape):
+            pass # output[index] = scipy.ndimage.map_coordinates(data[index], coords, order=order)
 
         return DataAndMetadata.new_data_and_metadata(
             data=output,
diff --git a/nion/data/test/Core_test.py b/nion/data/test/Core_test.py
index 3f9e236..901e302 100755
--- a/nion/data/test/Core_test.py
+++ b/nion/data/test/Core_test.py
@@ -1369,9 +1369,9 @@ class TestCore(unittest.TestCase):
 
         # Determine output grid shape
         if output_shape is None:
-            H, W = input_shape[-2:]
+            height, width = input_shape[-2:]
         else:
-            H, W = output_shape[-2:]
+            height, width = output_shape[-2:]
 
         # Create warp coordinates
         if identity:
@@ -1383,9 +1383,9 @@ class TestCore(unittest.TestCase):
             )
         else:
             # Resampling / scaling: map output grid into input index space
-            in_H, in_W = input_shape[-2:]
-            y = numpy.arange(0, in_H, in_H / H)
-            x = numpy.arange(0, in_W, in_W / W)
+            input_height, input_width = input_shape[-2:]
+            y = numpy.arange(0, input_height, input_height / height)
+            x = numpy.arange(0, input_width, input_width / width)
             warp_y, warp_x = numpy.meshgrid(y, x, indexing="ij")
 
         return src, [warp_y, warp_x]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants