InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

BUG: Make dict_from_transform more consistent with other dict representations #4635

Closed thewtex closed 2 months ago

thewtex commented 2 months ago

Encapsulate the "transformParameterization", "parametersValueType", "inputDimension", "outputDimension", which are static, into a "transformType" member, similar to "imageType", "meshType" for Images, Meshes. "transformParameterization" is the string name of the ITK transform class, less the trailing "Transform".

Example serialization:

{
    'transformType': {'transformParameterization': 'Translation', 'parametersValueType': 'float64', 'inputDimension': 3, 'outputDimension': 3},
    'name': '',
    'inputSpaceName': '',
    'outputSpaceName': '',
    'parameters': array([3., 2., 8.]),
    'fixedParameters': array([], dtype=float64),
    'numberOfParameters': 3,
    'numberOfFixedParameters': 0
}
[
    {
        'transformType': {'transformParameterization': 'Translation', 'parametersValueType': 'float64', 'inputDimension': 3, 'outputDimension': 3},
        'name': '',
        'inputSpaceName': '',
        'outputSpaceName': '',
        'parameters': array([3., 2., 8.]),
        'fixedParameters': array([], dtype=float64),
        'numberOfParameters': 3,
        'numberOfFixedParameters': 0
    },
    {
        'transformType': {'transformParameterization': 'Affine', 'parametersValueType': 'float64', 'inputDimension': 3, 'outputDimension': 3},
        'name': '',
        'inputSpaceName': '',
        'outputSpaceName': '',
        'parameters': array([  0.6563149 ,   0.58065837,  -0.48175367,  -0.74079868,
         0.37486398,  -0.55739959,  -0.14306664,   0.72271215,
         0.67617978, -66.        ,  69.        ,  32.        ]),
        'fixedParameters': array([0., 0., 0.]),
        'numberOfParameters': 12,
        'numberOfFixedParameters': 3
    }
]

Remove unused numpy import in setstate.

Support both a list of transforms and a single transform in dict_from_transform.

Add more smoke tests in extras.py.

PranjalSahu commented 2 months ago

Is it possible to share a sample json output (dict representation for a transform) for a single and composite transform? That will make these changes very clear.

Something similar to https://github.com/InsightSoftwareConsortium/ITK/pull/3470

PranjalSahu commented 2 months ago

I only have a question regarding the need to support a list of transforms in the dict_from_transform method. Since it already handles multiple transform use cases under the CompositeTransform Object.

Also supporting a list of transforms will make this different compared to Image and Mesh methods which only work on one Image/Mesh Object.

thewtex commented 2 months ago

Is it possible to share a sample json output (dict representation for a transform) for a single and composite transform? That will make these changes very clear.

Yes, added to the PR description and commit.

thewtex commented 2 months ago

I only have a question regarding the need to support a list of transforms in the dict_from_transform method. Since it already handles multiple transform use cases under the CompositeTransform Object.

Also supporting a list of transforms will make this different compared to Image and Mesh methods which only work on one Image/Mesh Object.

Yes, it is a bit different from Image/Mesh since we have a chain of transforms. Support for this use case came up in the test for working with the result of itk.transformread. We could have:

I added docstrings to help clarify.

PranjalSahu commented 2 months ago

LGTM