Skip to content

ENH: Add operator== and operator!= to itk::Image#2188

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Equal-Unequal-Image
Dec 19, 2020
Merged

ENH: Add operator== and operator!= to itk::Image#2188
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Equal-Unequal-Image

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker commented Dec 17, 2020

Added operator== and operator!= overloads which tell whether or not
two images are equal, based on their value semantics (not just their
addresses in memory).

Allowed testing if two images (not just image pointers) compare equal or
unequal by using GoogleTest EXPECT_EQ(image1, image2) and
EXPECT_NE(image1, image2).

@N-Dekker N-Dekker force-pushed the Equal-Unequal-Image branch from 3478e79 to 20f6b96 Compare December 17, 2020 08:57
@N-Dekker
Copy link
Copy Markdown
Contributor Author

N-Dekker commented Dec 17, 2020 •
edited
Loading

@hjmjohnson Thank you for the assignment! By the way, what does it mean to be assigned to a PR? pull_request::operator=(const Developer&)? 😄 Does it mean I'm now responsible for merging?

@hjmjohnson
Copy link
Copy Markdown
Member

hjmjohnson commented Dec 17, 2020

The assignment does not really mean much for ITK. I assigned it to indicate the most important person for this PR.

N-Dekker
N-Dekker commented Dec 17, 2020
View reviewed changes
Comment thread Modules/Core/Common/include/itkImage.h
@N-Dekker N-Dekker marked this pull request as ready for review December 17, 2020 22:31
@N-Dekker N-Dekker marked this pull request as draft December 17, 2020 23:07
@N-Dekker N-Dekker force-pushed the Equal-Unequal-Image branch from 20f6b96 to 9bd662b Compare December 17, 2020 23:32
@N-Dekker N-Dekker marked this pull request as ready for review December 17, 2020 23:35
@N-Dekker N-Dekker force-pushed the Equal-Unequal-Image branch from 9bd662b to 18bc959 Compare December 18, 2020 09:56
@N-Dekker
Copy link
Copy Markdown
Contributor Author

N-Dekker commented Dec 18, 2020 •
edited
Loading

Thanks for you approval, @dzenanz

FYI The force-push that I just did only cleans up the added tests a little bit. I just realized it should be sufficient to test two itk::Image types, instead of four.

@blowekamp
Copy link
Copy Markdown
Member

blowekamp commented Dec 18, 2020

This is nicely coded with nice tests. I have mixed opinions about the Image objects as a parameter being introduced, and introducing operators for the object not being the normal ITK style.

Aside from that, please look at the following place where ITK has already established a "tolerance" for comparison of the meta-data geometry:
https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/include/itkImageToImageFilter.hxx#L185-L221

You mention that the goal of this PR is to enable GTEST comparison operators. For that an approximation or tolerant testing method is really needed.

@N-Dekker
ENH: Add operator== and operator!= to itk::Image
895645b 
Added `operator==` and `operator!=` overloads which tell whether or not
two images are equal, based on their value semantics (not just their
addresses in memory).

Allowed testing if two images (not just image pointers) compare equal or
unequal by using GoogleTest `EXPECT_EQ(image1, image2)` and
`EXPECT_NE(image1, image2)`.
@N-Dekker N-Dekker force-pushed the Equal-Unequal-Image branch from 18bc959 to 895645b Compare December 18, 2020 19:51
dzenanz
dzenanz approved these changes Dec 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Sometimes, we might want true equality. And the equality operator a natural place for it.

We could add approximate equality method to Image? But that can be a later PR.

@hjmjohnson hjmjohnson merged commit 7f3337d into InsightSoftwareConsortium:master Dec 19, 2020
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Dec 22, 2020
@N-Dekker
ENH: Extend ITKWriteImageFunctionTest by reading the written image back
72ea76b 
Tests that an image written by `itk::WriteImage` can be read back,
without loss of data.

Uses two recently added functions:
 - `itk::ReadImage`: pull request InsightSoftwareConsortium#2184 commit aeea88b
 - `operator==` for images: pull request InsightSoftwareConsortium#2188 commit 7f3337d

(`operator==` is called internally by GoogleTest `EXPECT_EQ`.)
@N-Dekker N-Dekker mentioned this pull request Dec 22, 2020
Merged
hjmjohnson pushed a commit that referenced this pull request Dec 22, 2020
@N-Dekker @hjmjohnson
ENH: Extend ITKWriteImageFunctionTest by reading the written image back
21cf648 
Tests that an image written by `itk::WriteImage` can be read back,
without loss of data.

Uses two recently added functions:
 - `itk::ReadImage`: pull request #2184 commit aeea88b
 - `operator==` for images: pull request #2188 commit 7f3337d

(`operator==` is called internally by GoogleTest `EXPECT_EQ`.)
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jan 10, 2021
@N-Dekker
ENH: Add operator==, operator!= to MetaDataObject and MetaDataDictionary
b76cecc 
Follow-up to "ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d
@N-Dekker N-Dekker mentioned this pull request Jan 10, 2021
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jan 11, 2021
@N-Dekker
ENH: Add operator==, operator!= to MetaDataObject and MetaDataDictionary
ce8cf43 
Follow-up to "ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jan 11, 2021
@N-Dekker
ENH: Add operator==, operator!= to MetaDataObject and MetaDataDictionary
9cb80f0 
Follow-up to "ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d
dzenanz pushed a commit that referenced this pull request Apr 28, 2021
@N-Dekker @dzenanz
ENH: Add operator==, operator!= to MetaDataObject and MetaDataDictionary
694bbc0 
Follow-up to "ENH: Add operator== and operator!= to itk::Image"
pull request #2188
commit 7f3337d
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 7, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
98fc9d5 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 7, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
9c32216 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
@N-Dekker N-Dekker mentioned this pull request Jun 7, 2021
Merged
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 7, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
e809afc 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 13, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
b432b7a 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 14, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
4549ea3 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 14, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
b02c9cf 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 15, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
2e9f9ad 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request InsightSoftwareConsortium#2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue InsightSoftwareConsortium#2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
N-Dekker added a commit that referenced this pull request Jun 15, 2021
@N-Dekker
COMP: Support non-EqualityComparable pixels Image template instantiation
a293c03 
The ability to instantiate `itk::Image<TPixel, VImageDimension>` for a
non-EqualityComparable pixel type was broken by:

"ENH: Add operator== and operator!= to itk::Image"
pull request #2188
commit 7f3337d

Reported by Bradley Lowekamp:
issue #2583
"Explicit instantiation of itk::Image fails with 5.2 when pixel type
does not support operator=="

This commit fixes the issue by defining the `operator==` and `operator!=`
friend functions of `itk::Image` as function _templates_, rather than as
non-templates.
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

@blowekamp blowekamp Awaiting requested review from blowekamp

Assignees

@N-Dekker N-Dekker

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@N-Dekker @hjmjohnson @blowekamp @dzenanz