New Spatial Object migration guide needs expanding

[hjmjohnson]

Description

These changes needed to be made to compile the ANTs package. It would be nice if the migration guide were improved to indiate that these changes would be necessary.

       typename BlobType::PointType centerPoint;
       image->TransformIndexToPhysicalPoint(  cIter.GetIndex(), centerPoint );
       BlobPointer blob = BlobType::New();
-      blob->SetSigma( 1 );
+      blob->SetSigmaInObjectSpace( 1 );
       blob->SetScaleSpaceValue( val );
       blob->SetCenter(  cIter.GetIndex() );
       const typename BlobType::VectorType centerVector = centerPoint - zeroPoint;
-      blob->GetObjectToParentTransform()->SetOffset(centerVector);
-      blob->ComputeBoundingBox();
+      blob->GetModifiableObjectToParentTransform()->SetOffset(centerVector);
+      blob->ComputeMyBoundingBox();
       blobs1.push_back( blob );
       }
     }
     BlobPointer blob = BlobType::New();
-    blob->SetSigma( 1 );
+    blob->SetSigmaInObjectSpace( 1 );
     blob->SetScaleSpaceValue( image2->GetPixel( mIter.GetIndex() ) );
     blob->SetCenter(  mIter.GetIndex() );
     const typename BlobType::VectorType centerVector = centerPoint - zeroPoint;
-    blob->GetObjectToParentTransform()->SetOffset(centerVector);
-    blob->ComputeBoundingBox();
+    blob->GetModifiableObjectToParentTransform()->SetOffset(centerVector);
+    blob->ComputeMyBoundingBox();
     blobs2.push_back( blob );
     }

   {
-    return this->GetSigma() *  itk::Math::sqrt2;
+    return this->GetSigmaInObjectSpace() *  itk::Math::sqrt2;
   }

-    blob->GetObjectToParentTransform()->SetOffset(centerVector);
-    blob->ComputeBoundingBox();
+    blob->GetModifiableObjectToParentTransform()->SetOffset(centerVector);
+    blob->ComputeMyBoundingBox();

-            if( !fixedImageMask || fixedImageMask->IsInside( point ) )
+            if( !fixedImageMask || fixedImageMask->IsInsideInObjectSpace( point ) )

Additionally, I think these examples in ITK proper would not compile without changing to "GetModifiableObjectToParentTransform" so they should be removed, or fixed and tested.

Examples/RegistrationITKv3/ModelToImageRegistration1.cxx:  ellipse1->GetObjectToParentTransform()->SetOffset(offset);
Examples/RegistrationITKv3/ModelToImageRegistration1.cxx:  ellipse2->GetObjectToParentTransform()->SetOffset(offset);
Examples/RegistrationITKv3/ModelToImageRegistration1.cxx:  ellipse3->GetObjectToParentTransform()->SetOffset(offset);
Modules/Core/SpatialObjects/test/SpatialObjectTest.py:ellipse.GetObjectToParentTransform().SetOffset([20, 20])
Modules/Core/SpatialObjects/test/SpatialObjectTest.py:box.GetObjectToParentTransform().SetOffset([20, 40])
Modules/Core/SpatialObjects/test/SpatialObjectTest.py:gaussian.GetObjectToParentTransform().SetOffset([60, 60])

Steps to Reproduce

Try to compile ANTs with new version of ITK.

Expected behavior

If building with legacy mode, then the old function names should be available in deprecated mode issuing warnings.

Actual behavior

Developers spend a lot of time trying to figure out how to change code to meet the new naming conventions :).

Reproducibility

Always, until ANTs upgrades all code and requires ITK v5 after March 29th.

Versions

After update to SpatialObjects.

Environment

All

[hjmjohnson]

See: ANTsX/ANTs@21ba537

[hjmjohnson]

I am wondering if the following paradigm would be better:

blob->GetModifiableObjectToParentTransform()->SetOffset(centerVector);

as

blob->SetObjectToParentTransformOffset();

[hjmjohnson]

Was Modules/Core/SpatialObjects/include/itkCylinderSpatialObject.h file intended to be deleted? If so, that should be documented.

[hjmjohnson]

@aylward
#662 fails (a small test case on top of master)
#663 succeeds( Same as #662, but applied before spatial objects)

[aylward]

You need to call update() after calling SetImage() on the ImageMaskSpatialObject. This is in the new documentation. I will add it to the migration guide as well. S

…

--- Sent via phone.

On Sat, Mar 30, 2019, 5:30 PM Hans Johnson ***@***.***> wrote: @aylward <https://github.com/aylward> #662 <#662> fails (a small test case on top of master) #663 <#663> succeeds( Same as #662 <#662>, but applied before spatial objects) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#656 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAREr5XEcmTfxCH8-VKWit3_Ti6v0KEUks5vb9dogaJpZM4cTYqM> .

[hjmjohnson]

The return value of ComputeMyBoundingBox is a boolean, but it is not clear what the return value represents.

In the base class it is hardcoded to return false. Should ComputeMyBoundingBox return void? If not the return value meaning should be clarified.

[hjmjohnson]

Also related to #642 and #645.

[aylward]

Hi In another thread we're taking about making ComputeMyBoundingBox protected. We will have to change the return to throwing an exception. This happens when the object has an empty bounding box, e.g., if a point based spatial object had no points. I will add to the documentation this week. Need to give much more emphasis to the importance of calling Update() whenever any parameters of the Spatial Object are changed. Thanks for your persistence and patience!!! Stephen

…

--- Sent via phone.

On Sun, Mar 31, 2019, 10:06 AM Hans Johnson ***@***.***> wrote: Also related to #642 <#642> and #645 <#645>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#656 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAREr2ERPtd8HtoMunJ1hyB6y87ds86eks5vcMDJgaJpZM4cTYqM> .

[aylward]

That is such a common use case, perhaps that is useful. We would add get and set methods for offset, scale, and rotation. Sound right?

…

On Sat, Mar 30, 2019, 8:17 AM Hans Johnson ***@***.***> wrote: I am wondering if the following paradigm would be better: blob->GetModifiableObjectToParentTransform()->SetOffset(centerVector); as blob->SetObjectToParentTransformOffset(); — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#656 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAREr3nksJJtUWGxwjMcvWRQ4-udYlXTks5vb1XKgaJpZM4cTYqM> .

[hjmjohnson]

@aylward I'm working on a patch right now for making both ComputeMyBoundingBox and ComputeObjectToWorldTransform private and forcing them to use "Update" instead.