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: Add ImageBufferAndIndexRange example #368

Merged
merged 1 commit into from
May 21, 2022
Merged

ENH: Add ImageBufferAndIndexRange example #368

merged 1 commit into from
May 21, 2022

Conversation

N-Dekker
Copy link
Collaborator

Using ImageBufferRange and IndexRange, which were introduced with ITK 5.0.

@github-actions github-actions bot added area:Core Issues affecting the Core module area:Documentation Issues affecting the Documentation module language:C++ Changes to C++ examples type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels May 19, 2022
@N-Dekker N-Dekker force-pushed the ImageBufferAndIndexRange branch from 8659616 to cf25845 Compare May 19, 2022 21:46
@N-Dekker N-Dekker marked this pull request as ready for review May 19, 2022 21:47
@tbirdso tbirdso self-requested a review May 20, 2022 13:39
tbirdso
tbirdso reviewed May 20, 2022
View reviewed changes
Copy link
Contributor

@tbirdso tbirdso left a comment

Choose a reason for hiding this comment

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

@N-Dekker Looking great! One question below.

src/Core/Common/ImageBufferAndIndexRange/Code.cxx
image->Allocate();

const itk::ImageBufferRange<TImage> imageBufferRange(*image);
std::iota(imageBufferRange.begin(), imageBufferRange.end(), PixelType{ 0 });
Copy link
Contributor

Choose a reason for hiding this comment

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

Very cool!

src/Core/Common/ImageBufferAndIndexRange/Code.cxx Outdated Show resolved Hide resolved
@tbirdso
Copy link
Contributor

tbirdso commented May 20, 2022

Two CDash failures here

We may have difficulty in using classes in documentation that have been introduced after the 5.2 release, see #362 discussion.

@N-Dekker
Copy link
Collaborator Author

N-Dekker commented May 20, 2022 •
edited
Loading

Two CDash failures here

We may have difficulty in using classes in documentation that have been introduced after the 5.2 release, see #362 discussion.

ITKEx-build/src/Core/Common/ImageBufferAndIndexRange/Documentation.rst:57: WARNING: doxygenclass: Cannot find class "itk::ImageRegionIndexRange" in doxygen xml output for project "ITK" from directory: /.../bld/ITKEx-build/ITKDoxygenXML

"itk::ImageRegionIndexRange" is an alias, not a class. Maybe that's the problem...?

"itk::ImageRegionIndexRange" is at https://itk.org/Doxygen/html/namespaceitk.html#a6149e4650b1896fdc969600c22eb50f4 defined as:

template<unsigned int VDimension>
using itk::ImageRegionIndexRange = typedef IndexRange<VDimension, false>

With my upcoming amend + force-push, I think I'll just remove "ImageRegionIndexRange" from the breathelink, in src/Core/Common/ImageBufferAndIndexRange/Documentation.rst

@N-Dekker N-Dekker marked this pull request as draft May 20, 2022 14:08
@N-Dekker N-Dekker force-pushed the ImageBufferAndIndexRange branch from cf25845 to 68e7d3d Compare May 20, 2022 14:58
thewtex
thewtex approved these changes May 20, 2022
View reviewed changes
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

👨‍🎤 😎 awesome!!

@N-Dekker
Copy link
Collaborator Author

N-Dekker commented May 20, 2022 •
edited
Loading

Unfortunately "build-test-documentation (ubuntu-18.04)" still fails, even after my force-push. Any idea why?

It says at https://open.cdash.org/viewBuildError.php?type=1&buildid=7925758 :

checking consistency... /.../bld/ITKEx-build/src/Core/Common/ImageBufferAndIndexRange/Documentation.rst: WARNING: document isn't included in any toctree

@N-Dekker N-Dekker force-pushed the ImageBufferAndIndexRange branch 2 times, most recently from 3979ef1 to 3a78d54 Compare May 21, 2022 09:00
@N-Dekker
Copy link
Collaborator Author

FYI The force-push that I just did adjusted the PrintPixelValues example, because I did not like calling image.GetPixel(index) inside the for loop anymore. Because of course, image.GetPixel(index) is inefficient, when it is called for a whole range of consecutive pixels. While this is just example code, ITK users might simply copy the code for their applications and do such an iteration on larger images. So I thought, as an example, better use a range-based for loop over the image buffer range. 😃

@N-Dekker N-Dekker marked this pull request as ready for review May 21, 2022 10:42
@N-Dekker
ENH: Add ImageBufferAndIndexRange example
0ba2890 
Using `ImageBufferRange` and `IndexRange`, which were introduced with ITK 5.0.
@N-Dekker N-Dekker force-pushed the ImageBufferAndIndexRange branch from 3a78d54 to 0ba2890 Compare May 21, 2022 10:55
thewtex
thewtex approved these changes May 21, 2022
View reviewed changes
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Awesome, Neils!

@thewtex thewtex merged commit 1284f09 into InsightSoftwareConsortium:master May 21, 2022
@tbirdso
Copy link
Contributor

tbirdso commented May 23, 2022

🚀 Excellent example Niels! https://examples.itk.org/src/core/common/imagebufferandindexrange/documentation

@N-Dekker
Copy link
Collaborator Author

@tbirdso @thewtex Thanks for your encouragements and approvals. Still a very small improvement, adjusting the title of the example: pull request #386 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbirdso tbirdso tbirdso left review comments

@thewtex thewtex thewtex approved these changes

Assignees
No one assigned
Labels
area:Core Issues affecting the Core module area:Documentation Issues affecting the Documentation module language:C++ Changes to C++ examples type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@N-Dekker @tbirdso @thewtex