Skip to content
This repository was archived by the owner on May 27, 2026. It is now read-only.

ENH: Add (not)wrapping for Iterator-like classes.#63

Merged
phcerdan merged 1 commit into
InsightSoftwareConsortium:masterfrom
jhlegarreta:AddIteratorClassesWrapping
Aug 20, 2017
Merged

ENH: Add (not)wrapping for Iterator-like classes.#63
phcerdan merged 1 commit into
InsightSoftwareConsortium:masterfrom
jhlegarreta:AddIteratorClassesWrapping

Conversation

@jhlegarreta

Copy link
Copy Markdown
Member

ITK iterator class wrapping has no use, but corresponding files with
.notwrapped extension are added with a message warning about it are added
so that developers know about this.

@jhlegarreta

Copy link
Copy Markdown
Member Author

Follow-up of comment on Iterator-like classes in issue #56.

@phcerdan

Copy link
Copy Markdown
Collaborator

@jhlegarreta Probably doesn't matter, but maybe for consistency we can follow the existing notwrapping style for iterators:

https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/wrapping/itkImageConstIteratorWithIndex.notwrapped

itk_wrap_class("itk::ImageRegionConstIteratorWithIndex")
  itk_wrap_image_filter("${WRAP_ITK_ALL_TYPES}" 1)
itk_end_wrap_class()

@jhlegarreta

Copy link
Copy Markdown
Member Author

@phcerdan Yes, I was in two minds when doing this, but @thewtex recommended in #56 to add

a comment that explains this may help others in the future.

But it is highly likely that I misinterpreted what he meant. Matt, does what I did match what you expected, or else, should I change to/add the exiting convention for (not)wrapping iterators?

Thanks.

@thewtex

thewtex commented Aug 16, 2017

Copy link
Copy Markdown
Member

It may be clearer to have the file contents only as

message(FATAL_ERROR "No use to wrapping the iterator.")

and also fix itkImageConstIteratorWithIndex.notwrapped.

ITK iterator class wrapping has no use, but corresponding files with
.notwrapped extension are added with a message warning about it are added
so that developers know about this.
@jhlegarreta
jhlegarreta force-pushed the AddIteratorClassesWrapping branch from b29da8c to 25786a4 Compare August 17, 2017 15:13
@jhlegarreta

Copy link
Copy Markdown
Member Author

Done. Submitted a topic to gerrit for itkImageConstIteratorWithIndex.notwrapped.

@phcerdan
phcerdan merged commit 2e36fea into InsightSoftwareConsortium:master Aug 20, 2017
kwrobot referenced this pull request in Kitware/ITK Sep 12, 2017
Following a discussion on wrapping ITK iterators on a remote module,
https://github.com/phcerdan/ITKIsotropicWavelets/pull/63

Matt suggested changing the itk::ImageConstIteratorWithIndex wrapping to
display an error message only.

This patch set changes the the Iterator classes' *.notwrapped files to
contain only an error message.

Change-Id: I9c7d20ef6d07f7af66600966acfc5a92fce3ddf7
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants