Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815

N-Dekker · 2024-08-22T12:36:15Z

Documented that Iterator(image, region) constructors initialize the iterator at the beginning of the region. Added a GoogleTest unit test to checks this added specification.

Checks that an iterator, constructed by `IteratorType(image, region)` is at the begin. Checks 22 different iterator types from Core/Common.

Explicitly specify that those constructors initialize the iterator at the begin of the region.

It is unnecessary to call GoToBegin() on an ITK iterator when it has just been constructed, by `Iterator(image, region)`. Such a function call just takes compile-time and run-time. Supported by ITK pull request InsightSoftwareConsortium/ITK#4815 "Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test"

N-Dekker · 2024-08-26T16:56:49Z

Modules/Core/Common/test/itkImageRegionIteratorGTest.cxx

+// Has_IsAtBegin tells whether the specified iterator type has an `IsAtBegin()` member function, returning a `bool`. It
+// is inspired by "How to detect whether there is a specific member variable in class?", answered by Andy Prowl, Jan 25,
+// 2013 at https://stackoverflow.com/a/14523787 (the `has_id` example).
+template <typename TIterator, typename = bool>
+struct Has_IsAtBegin : std::false_type
+{};
+
+template <typename TIterator>
+struct Has_IsAtBegin<TIterator, decltype(std::declval<TIterator>().IsAtBegin())> : std::true_type
+{};


Fun fact: I found this "has member" trick (Has_IsAtBegin) kind-of independent of has_Print and has_output_operator, from Bradley's (@blowekamp) pull requests, BUG: Fix printing of values in MetaDataObject #4814 and STYLE: move details to MetaDataObjectDetail #4821

For the record,

The direct link to the StackOverflow post that I read for this pull request: "How to detect whether there is a specific member variable in class?", answered by Andy Prowl, Jan 25, 2013

has_Print and has_output_operator are at

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

Lines 31 to 46 in f66d9a4

template <class T, class = void>

struct has_Print : std::false_type

{};

template <class T>

struct has_Print<T, std::void_t<decltype(std::declval<T>().Print(std::declval<std::ostream &>()))>> : std::true_type

{};

template <class T, class = void>

struct has_output_operator : std::false_type

{};

template <class T>

struct has_output_operator<T, std::void_t<decltype(std::declval<std::ostream &>() << std::declval<T>())>>

: std::true_type

{};

Anyway, this Has_IsAtBegin<TIterator> meta-function is just an implementation detail of the unit test that I'm proposing. As a whole, I think this pull request is ready 😃

It is unnecessary to call GoToBegin() on an ITK iterator when it has just been constructed, by `Iterator(image, region)`. Such a function call just takes compile-time and run-time. Supported by ITK pull request InsightSoftwareConsortium/ITK#4815 "Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test"

blowekamp

LGTM. Just a commented on a small thought.

blowekamp · 2024-08-28T12:35:09Z

Modules/Core/Common/test/itkImageRegionIteratorGTest.cxx

+
+
+// Checks that an iterator that is just constructed by `IteratorType(image, region)` is at the begin.
+TEST(ImageRegionIterator, IsConstructedAtBegin)


The is some 💪 use of variadict templates here. However with the nested dispatching of the functions how does test report a failure? Does it indicate the type? Is some Information needed the the "EXPECT" statements?

Thanks, here is my reply: #4815 (comment)

Modules/Core/Common/test/itkImageRegionIteratorGTest.cxx

- Follow-up to pull request InsightSoftwareConsortium#4815 commit 66e6d8b "ENH: Add GoogleTest unit test `ImageRegionIterator.IsConstructedAtBegin`"

Explicitly specify that those constructors from iterator types of Modules/Filtering/ImageFrequency initialize the iterator at the begin of the region. - Follow-up to pull request InsightSoftwareConsortium#4815 commit 9eb1359 "DOC: Document Iterator(image, region) constructors initializing at begin"

N-Dekker · 2024-08-29T21:20:10Z

For the record, this pull request is meant to support style commits like pull request Remove unnecessary and duplicate iterator GoToBegin() calls from Modules/Filtering #3953 commit 5a60ed0

When an ITK iterator is just constructed by `IteratorType(image, region)`, it is already initialized at the begin of the region, as was tested and documented by pull request InsightSoftwareConsortium#4815 commit 66e6d8b and 9eb1359. - Follow-up to pull request InsightSoftwareConsortium#3953 commit 5a60ed0 "STYLE: Remove unnecessary iterator `GoToBegin()` calls from Filtering", 7 March 2023

- Follow-up to pull request InsightSoftwareConsortium#4815 commit 66e6d8b "ENH: Add GoogleTest unit test `ImageRegionIterator.IsConstructedAtBegin`" Note: `FrequencyImageRegionIteratorWithIndex` and `FrequencyImageRegionConstIteratorWithIndex` are excluded from this commit, because they do not yet compile.

Explicitly specify that those constructors from iterator types of Modules/Filtering/ImageFrequency initialize the iterator at the begin of the region. Note: `FrequencyImageRegionIteratorWithIndex` and `FrequencyImageRegionConstIteratorWithIndex` are excluded from this commit, because they do not yet compile. - Follow-up to pull request InsightSoftwareConsortium#4815 commit 9eb1359 "DOC: Document Iterator(image, region) constructors initializing at begin"

When an ITK iterator is just constructed by `IteratorType(image, region)`, it is already initialized at the begin of the region, as was tested and documented by pull request #4815 commit 66e6d8b and 9eb1359. - Follow-up to pull request #3953 commit 5a60ed0 "STYLE: Remove unnecessary iterator `GoToBegin()` calls from Filtering", 7 March 2023

- Follow-up to pull request #4815 commit 66e6d8b "ENH: Add GoogleTest unit test `ImageRegionIterator.IsConstructedAtBegin`" Note: `FrequencyImageRegionIteratorWithIndex` and `FrequencyImageRegionConstIteratorWithIndex` are excluded from this commit, because they do not yet compile.

Explicitly specify that those constructors from iterator types of Modules/Filtering/ImageFrequency initialize the iterator at the begin of the region. Note: `FrequencyImageRegionIteratorWithIndex` and `FrequencyImageRegionConstIteratorWithIndex` are excluded from this commit, because they do not yet compile. - Follow-up to pull request #4815 commit 9eb1359 "DOC: Document Iterator(image, region) constructors initializing at begin"

github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module labels Aug 22, 2024

dzenanz approved these changes Aug 22, 2024

View reviewed changes

N-Dekker force-pushed the GTest-ImageRegionIterator-CheckBeginAndEnd branch from c02faaa to bc2a55e Compare August 22, 2024 17:32

N-Dekker changed the title ~~Document that ImageRegionIterator constructors initialize at the begin, add GTest unit test~~ Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test Aug 22, 2024

N-Dekker added 2 commits August 24, 2024 13:11

ENH: Add GoogleTest unit test ImageRegionIterator.IsConstructedAtBegin

c5d211c

Checks that an iterator, constructed by `IteratorType(image, region)` is at the begin. Checks 22 different iterator types from Core/Common.

DOC: Document Iterator(image, region) constructors initializing at begin

88b854f

Explicitly specify that those constructors initialize the iterator at the begin of the region.

N-Dekker force-pushed the GTest-ImageRegionIterator-CheckBeginAndEnd branch from bc2a55e to 88b854f Compare August 24, 2024 11:12

N-Dekker marked this pull request as ready for review August 26, 2024 13:32

N-Dekker mentioned this pull request Aug 26, 2024

Remove GoToBegin() calls after iterator construction, move iterator variable declarations into init-statement of for SuperElastix/elastix#1218

Merged

N-Dekker commented Aug 26, 2024

View reviewed changes

dzenanz approved these changes Aug 26, 2024

View reviewed changes

blowekamp approved these changes Aug 28, 2024

View reviewed changes

N-Dekker commented Aug 28, 2024

View reviewed changes

Modules/Core/Common/test/itkImageRegionIteratorGTest.cxx Show resolved Hide resolved

dzenanz merged commit 9eb1359 into InsightSoftwareConsortium:master Aug 28, 2024

N-Dekker mentioned this pull request Aug 29, 2024

Document that Iterator(image, region) constructors of "ImageFrequency" initialize at the begin, add GTest unit test #4827

Merged

N-Dekker mentioned this pull request Aug 30, 2024

STYLE: Remove unnecessary iterator GoToBegin() calls from Core #4830

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815

Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815

Uh oh!

N-Dekker commented Aug 22, 2024 •

edited

Loading

Uh oh!

N-Dekker Aug 26, 2024 •

edited

Loading

Uh oh!

N-Dekker Aug 28, 2024

Uh oh!

blowekamp left a comment

Uh oh!

blowekamp Aug 28, 2024

Uh oh!

N-Dekker Aug 28, 2024

Uh oh!

Uh oh!

N-Dekker commented Aug 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	template <class T, class = void>
	struct has_Print : std::false_type
	{};

	template <class T>
	struct has_Print<T, std::void_t<decltype(std::declval<T>().Print(std::declval<std::ostream &>()))>> : std::true_type
	{};

	template <class T, class = void>
	struct has_output_operator : std::false_type
	{};

	template <class T>
	struct has_output_operator<T, std::void_t<decltype(std::declval<std::ostream &>() << std::declval<T>())>>
	: std::true_type
	{};



		// Checks that an iterator that is just constructed by `IteratorType(image, region)` is at the begin.
		TEST(ImageRegionIterator, IsConstructedAtBegin)

Uh oh!

Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815

Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815

Uh oh!

Conversation

N-Dekker commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

N-Dekker Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

N-Dekker Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

blowekamp left a comment

Choose a reason for hiding this comment

Uh oh!

blowekamp Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

N-Dekker Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

N-Dekker commented Aug 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

N-Dekker commented Aug 22, 2024 •

edited

Loading

N-Dekker Aug 26, 2024 •

edited

Loading