InsightSoftwareConsortium / ITKIsotropicWavelets

External Module for ITK, implementing Isotropic Wavelets and Riesz Filter for multiscale phase analysis.
Apache License 2.0
13 stars 11 forks source link

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

Closed jhlegarreta closed 7 years ago

jhlegarreta commented 7 years ago

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 commented 7 years ago

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

phcerdan commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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.

jhlegarreta commented 7 years ago

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