-
-
Notifications
You must be signed in to change notification settings - Fork 708
Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815
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.
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
Document that Iterator(image, region) constructors initialize at the begin, add GTest unit test #4815
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| /*========================================================================= | ||
| * | ||
| * Copyright NumFOCUS | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0.txt | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| *=========================================================================*/ | ||
|
|
||
| // First include the header files to be tested: | ||
| #include "itkImageConstIterator.h" | ||
| #include "itkImageConstIteratorWithIndex.h" | ||
| #include "itkImageConstIteratorWithOnlyIndex.h" | ||
| #include "itkImageIterator.h" | ||
| #include "itkImageIteratorWithIndex.h" | ||
| #include "itkImageLinearConstIteratorWithIndex.h" | ||
| #include "itkImageLinearIteratorWithIndex.h" | ||
| #include "itkImageRegionConstIterator.h" | ||
| #include "itkImageRegionConstIteratorWithIndex.h" | ||
| #include "itkImageRegionConstIteratorWithOnlyIndex.h" | ||
| #include "itkImageRegionExclusionConstIteratorWithIndex.h" | ||
| #include "itkImageRegionExclusionIteratorWithIndex.h" | ||
| #include "itkImageRegionIterator.h" | ||
| #include "itkImageRegionIteratorWithIndex.h" | ||
| #include "itkImageRegionReverseConstIterator.h" | ||
| #include "itkImageRegionReverseIterator.h" | ||
| #include "itkImageReverseConstIterator.h" | ||
| #include "itkImageReverseIterator.h" | ||
| #include "itkImageScanlineConstIterator.h" | ||
| #include "itkImageScanlineIterator.h" | ||
| #include "itkImageSliceConstIteratorWithIndex.h" | ||
| #include "itkImageSliceIteratorWithIndex.h" | ||
|
|
||
| #include "itkImage.h" | ||
| #include <gtest/gtest.h> | ||
| #include <type_traits> // For false_type and true_type. | ||
|
|
||
| namespace | ||
| { | ||
| // 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 | ||
| {}; | ||
|
|
||
|
|
||
| template <typename TIterator> | ||
| void | ||
| CheckConstructedAtBegin() | ||
| { | ||
| using ImageType = typename TIterator::ImageType; | ||
| using IndexType = typename TIterator::IndexType; | ||
| using SizeType = typename TIterator::SizeType; | ||
| using RegionType = typename TIterator::RegionType; | ||
|
|
||
| const auto image = ImageType::New(); | ||
|
|
||
| // Use a small image size, so that the unit test won't take a long time. | ||
| static constexpr itk::SizeValueType imageSizeValue{ 4 }; | ||
|
|
||
| image->SetRegions(SizeType::Filled(imageSizeValue)); | ||
| image->Allocate(); | ||
|
|
||
| // Check various regions, specified by the following `indexValue` and `sizeValue` combinations: | ||
| for (const itk::IndexValueType indexValue : { 0, 1 }) | ||
| { | ||
| for (const auto sizeValue : { itk::SizeValueType{ 1 }, imageSizeValue - 1 }) | ||
| { | ||
| const RegionType imageRegion(IndexType::Filled(indexValue), SizeType::Filled(sizeValue)); | ||
|
|
||
| const TIterator iterator(image, imageRegion); | ||
| TIterator iteratorThatGoesToBegin = iterator; | ||
| iteratorThatGoesToBegin.GoToBegin(); | ||
| EXPECT_EQ(iterator, iteratorThatGoesToBegin); | ||
blowekamp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if constexpr (Has_IsAtBegin<TIterator>::value) | ||
| { | ||
| // Extra check, using IsAtBegin(), if the iterator has that member function. (Some iterators | ||
| // do not have an IsAtBegin() member function, for example, ImageRegionConstIteratorWithIndex.) | ||
| EXPECT_TRUE(TIterator(image, imageRegion).IsAtBegin()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| template <template <typename> typename... TIteratorTemplate> | ||
| void | ||
| CheckIteratorsConstructedAtBegin() | ||
| { | ||
| (CheckConstructedAtBegin<TIteratorTemplate<itk::Image<int>>>(), ...); | ||
| (CheckConstructedAtBegin<TIteratorTemplate<itk::Image<double, 3>>>(), ...); | ||
| } | ||
| } // namespace | ||
|
|
||
|
|
||
| // Checks that an iterator that is just constructed by `IteratorType(image, region)` is at the begin. | ||
| TEST(ImageRegionIterator, IsConstructedAtBegin) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, here is my reply: #4815 (comment) |
||
| { | ||
| CheckIteratorsConstructedAtBegin<itk::ImageConstIterator, | ||
| itk::ImageConstIteratorWithIndex, | ||
| itk::ImageConstIteratorWithOnlyIndex, | ||
| itk::ImageIterator, | ||
| itk::ImageIteratorWithIndex, | ||
| itk::ImageLinearConstIteratorWithIndex, | ||
| itk::ImageLinearIteratorWithIndex, | ||
| itk::ImageRegionConstIterator, | ||
| itk::ImageRegionConstIteratorWithIndex, | ||
| itk::ImageRegionConstIteratorWithOnlyIndex, | ||
| itk::ImageRegionExclusionConstIteratorWithIndex, | ||
| itk::ImageRegionExclusionIteratorWithIndex, | ||
| itk::ImageRegionIterator, | ||
| itk::ImageRegionIteratorWithIndex, | ||
| itk::ImageRegionReverseConstIterator, | ||
| itk::ImageRegionReverseIterator, | ||
| itk::ImageReverseConstIterator, | ||
| itk::ImageReverseIterator, | ||
| itk::ImageScanlineConstIterator, | ||
| itk::ImageScanlineIterator, | ||
| itk::ImageSliceConstIteratorWithIndex, | ||
| itk::ImageSliceIteratorWithIndex>(); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has_IsAtBegin) kind-of independent ofhas_Printandhas_output_operator, from Bradley's (@blowekamp) pull requests, BUG: Fix printing of values in MetaDataObject #4814 and STYLE: move details to MetaDataObjectDetail #4821For the record,
has_Printandhas_output_operatorare atITK/Modules/Core/Common/include/itkMetaDataObjectDetail.h
Lines 31 to 46 in f66d9a4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😃