Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ENH: Convenience overloads for ImageBase Transform member functions #868

Merged
merged 1 commit into from
May 16, 2019
Merged

ENH: Convenience overloads for ImageBase Transform member functions #868

merged 1 commit into from
May 16, 2019

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented May 14, 2019 •
edited
Loading

Added convenience overloads to the six ImageBase Transform member
functions, returning the result by value, instead of having an output
parameter:

TransformPhysicalPointToIndex
TransformPhysicalPointToContinuousIndex
TransformContinuousIndexToPhysicalPoint
TransformIndexToPhysicalPoint
TransformLocalVectorToPhysicalVector
TransformPhysicalVectorToLocalVector

These overloads allow users to have a more "functional" programming
style, or at least, place the transformation result in a const variable.

So instead of:

Bar bar;
image->TransformFooToBar(foo, bar);

It typically allows users to write:

const auto bar = image->TransformFooToBar(foo);

Added a unit test to itkImageBaseGTest

@dzenanz
Copy link
Member

dzenanz commented May 14, 2019

I have been annoyed by absence of these more than once!

hjmjohnson
hjmjohnson reviewed May 14, 2019
View reviewed changes
Modules/Core/Common/include/itkImageBase.h
* Floating point index results are rounded to integers
* Returns true if the resulting index is within the image, false otherwise
Copy link
Member

Choose a reason for hiding this comment

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

Minor changes request to documentation. I think it is worth explicitly noting that these functions do not verify that the resulting indices fall within the image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hjmjohnson Please check the added note:

ITK/Modules/Core/Common/include/itkImageBase.h

Line 403 in ec7e476

* \note This specific overload does not figure out whether or not

ITK/Modules/Core/Common/include/itkImageBase.h

Line 452 in ec7e476

* \note This specific overload does not figure out whether or not
(copied from the previous note, but adapted code snippets)

hjmjohnson
hjmjohnson requested changes May 14, 2019
View reviewed changes
Copy link
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

+100 I too have desired these convenience methods for some time.

I have a minor comment change request.

@N-Dekker
Copy link
Contributor Author

Update: Still preparing an amend that should address the comment by @hjmjohnson and add at least a very little unit test. Hopefully tomorrow... :-)

@hjmjohnson
Copy link
Member

I approve the wording of the commit message.

@N-Dekker
Copy link
Contributor Author

N-Dekker commented May 15, 2019 •
edited
Loading

The GTest that I added compiles well on ITK.Windows, but it fails on macOS and Linux, because I should have added a template keyword on various places. For example:

imageBase.TransformPhysicalPointToContinuousIndex<float>(PointType())

Should be:

imageBase.template TransformPhysicalPointToContinuousIndex<float>(PointType())

At line

ITK/Modules/Core/Common/test/itkImageBaseGTest.cxx

Line 66 in ec7e476

imageBase.TransformPhysicalPointToContinuousIndex<float>(PointType()),

I'll try to fix it (tomorrow).

@N-Dekker
ENH: Convenience overloads for ImageBase Transform member functions
6b1f1e8 
Added convenience overloads to the six `ImageBase` Transform member
functions, returning the result by value, instead of having an output
parameter:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint
    TransformLocalVectorToPhysicalVector
    TransformPhysicalVectorToLocalVector

These overloads allow users to have a more "functional" programming
style, or at least, place the transformation result in a `const` variable.

So instead of:

    Bar bar;
    image->TransformFooToBar(foo, bar);

It typically allows users to write:

    const auto bar = image->TransformFooToBar(foo);

Added a unit test to itkImageBaseGTest.
@N-Dekker
Copy link
Contributor Author

I think this pull request is OK now. CDash appears stuck "in progress" for no reason. It has succeeded at Linux, Windows and macOS. :-)

@dzenanz dzenanz merged commit 65233e3 into InsightSoftwareConsortium:master May 16, 2019
@N-Dekker N-Dekker mentioned this pull request Jun 5, 2019
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Apr 4, 2021
@N-Dekker
ENH: PhasedArray3DSpecialCoordinatesImage TransformPhysicalPointTo...
bc71e4c 
Added `TransformPhysicalPointToIndex(point)` and
`TransformPhysicalPointToContinuousIndex(point)` overloads to
`PhasedArray3DSpecialCoordinatesImage`, which return the index,
instead of a `bool`.

Follow-up to:
pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Apr 4, 2021
@N-Dekker
ENH: Overload PhasedArray3DSpecialCoordinatesImage Transform functions
87f5653 
Added convenience overloads of four Transform member functions of
`PhasedArray3DSpecialCoordinatesImage`, returning the result by value,
instead of having an output parameter:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Follow-up to:
pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
dzenanz pushed a commit that referenced this pull request Apr 6, 2021
@N-Dekker @dzenanz
ENH: Overload PhasedArray3DSpecialCoordinatesImage Transform functions
3d7b84e 
Added convenience overloads of four Transform member functions of
`PhasedArray3DSpecialCoordinatesImage`, returning the result by value,
instead of having an output parameter:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Follow-up to:
pull request #868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 20, 2021
@N-Dekker
BUG: Add missing single parameter conversion functions to ImageAdaptor
b65404e 
Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 20, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
ae6cb12 
Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 20, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
5e324f9 
Added the five missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0.

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
@N-Dekker N-Dekker mentioned this pull request Nov 20, 2021
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 20, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
b8a4f16 
Added the five missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0.

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 21, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
c69ee92 
Added the five missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0.

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 21, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
ca99d5a 
Added the four missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 21, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
a798cf8 
Added the four missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"

And pull request InsightSoftwareConsortium#1207
commit 55f8ac2
"BUG: Missing ImageAdapter function signatures"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 21, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
810d2ea 
Added the four missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"

And pull request InsightSoftwareConsortium#1207
commit 55f8ac2
"BUG: Missing ImageAdapter function signatures"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 21, 2021
@N-Dekker
BUG: Add missing single argument ImageAdaptor Transform member functions
5fa53b4 
Added the four missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Included a GoogleTest unit test.

Follow-up to pull request InsightSoftwareConsortium#868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"

And pull request InsightSoftwareConsortium#1207
commit 55f8ac2
"BUG: Missing ImageAdapter function signatures"
dzenanz pushed a commit that referenced this pull request Nov 23, 2021
@N-Dekker @dzenanz
BUG: Add missing single argument ImageAdaptor Transform member functions
2d06a6a 
Added the four missing single parameter overloads of point/index/vector
conversion member functions to `ImageAdaptor`, which were already there
for `ImageBase` with ITK version 5.0:

    TransformPhysicalPointToIndex
    TransformPhysicalPointToContinuousIndex
    TransformContinuousIndexToPhysicalPoint
    TransformIndexToPhysicalPoint

Included a GoogleTest unit test.

Follow-up to pull request #868
commit 65233e3
"ENH: Convenience overloads for ImageBase Transform member functions"

And pull request #1207
commit 55f8ac2
"BUG: Missing ImageAdapter function signatures"
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Feb 4, 2022
@N-Dekker
STYLE: Let TransformPointToContinuousGridIndex return the index by value
8484f28 
Return-by-value is easier to read than pass-by-non-const-reference, and `ContinuousIndexType` has lightweight copy semantics, so it appears preferable for `AdvancedBSplineDeformableTransformBase::TransformPointToContinuousGridIndex` to simply return the continuous index.

Declared local continuous index variables `const`.

Analogous to ITK pull request InsightSoftwareConsortium/ITK#868 commit InsightSoftwareConsortium/ITK@65233e3 "ENH: Convenience overloads for ImageBase Transform member functions"
@N-Dekker N-Dekker mentioned this pull request Feb 4, 2022
Merged
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Feb 5, 2022
@N-Dekker
STYLE: Let TransformPointToContinuousGridIndex return the index by value
c7fb15d 
Return-by-value is easier to read than pass-by-non-const-reference, and `ContinuousIndexType` has lightweight copy semantics, so it appears preferable for `AdvancedBSplineDeformableTransformBase::TransformPointToContinuousGridIndex` to simply return the continuous index.

Declared local continuous index variables `const`.

Analogous to ITK pull request InsightSoftwareConsortium/ITK#868 commit InsightSoftwareConsortium/ITK@65233e3 "ENH: Convenience overloads for ImageBase Transform member functions"
@N-Dekker N-Dekker mentioned this pull request Apr 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjmjohnson hjmjohnson hjmjohnson approved these changes

@dzenanz dzenanz dzenanz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@N-Dekker @dzenanz @hjmjohnson