ITK icon indicating copy to clipboard operation
ITK copied to clipboard

Overload the stream insertion operator for types that are missing it

Open jhlegarreta opened this issue 5 years ago • 10 comments

Description

Overload the stream insertion operator for types that are missing it.

Expected behavior

To be able to print any data structure with a single line, e.g.:

os << indent << "MyStructure : " << m_MyStructure << std::endl;

Actual behavior

Developer's are forced to iterate over such structures to get them printed (usually in the PrintSelf method), e.g.:

  while( it != m_CirclesList.end() )
    {
    os << indent << "[" << i << "]: " << *it << std::endl;
    ++it;
    ++i;
    }

Additional information

Some fundamental ITK types (e.g. IndexTypes, SizeType, SpacingType, OffsetType, DirectionType, OffsetTable, RegionType) either seem not to have such an overload or it is just because some filters (itkImageConstIteratorWithIndex.h, itkImageIORegion.h, itkNeighborhood.h, itkImportImageFilter.h, itkConstNeighborhoodIterator.h, itkImportImageFilter.hxx, itkDisplacementFieldJacobianDeterminantFilter.h, itkGaussianBlurImageFunction.h, itkEllipsoidInteriorExteriorSpatialFunction.h) that declare ivars of such types are not using the correct types, since it looks weird that such fundamental types do not have the stream insertion operator overload.

Other more exotic type aliases (GradientType, StrideTable, SeedsContainerType, WeightsType, ErrorArrayType, ExtentArrayType) would also deserve some investigation, as well as other aliases that are not consistent with the toolkit convention for the intended use (e.g. InputType in itkEllipsoidInteriorExteriorSpatialFunction.h).

Finally, other data structures that are also missing such an overload are regular arrays, e.g.

double m_Sigma[ImageDimension2];

in itkBSplineControlPointImageFunction.h or itkGaussianDerivativeImageFuncton.h

or

double          m_Scale[NDimensions];

in itkScalableAffineTransform.h.

std::list types (e.g. itkHoughTransform2DCirclesImageFilter.h or itkHoughTransform2DLinesImageFilter.h).

Or vnl_matrix matrix (e.g. itkBSplineScatteredDataPointSetToImageFilter.h).

This is related to #512.

jhlegarreta avatar Feb 16 '19 23:02 jhlegarreta

@dzenanz, is this issue still relevant? If so I'd like to have a go at it. After listening to @thewtex talk about the library on cppcast today I got inspired to help out.

ChristianJacobsen avatar Aug 27 '21 16:08 ChristianJacobsen

Yes, this is still relevant.

dzenanz avatar Aug 27 '21 17:08 dzenanz

Looking forward to reviewing the contribution!

dzenanz avatar Aug 27 '21 17:08 dzenanz

Hi again @dzenanz!

As far as I am able to see the problems with printing fundamental types from itkIntTypes.h and itkFloatTypes.h no longer exist. Compiler seemed happy printing ImageConstIteratorWithIndex::GetIndex() returning an IndexType.

As for native arrays and std::list, I have so far added support to itkPrintHelper.h for those.

vnl_matrix already has an operator<< overload, so I'm leaving that one as well.

W.r.t.

Other more exotic type aliases

can you give me some examples of where they are printed currently and how you would like them printed/formatted?

ChristianJacobsen avatar Aug 31 '21 16:08 ChristianJacobsen

Example of an exotic type: GradientType: 1 2 3

A large part of the job is finding instances of these types and replacing custom printing operators by the new specialized overloads.

dzenanz avatar Aug 31 '21 18:08 dzenanz

And of course printing instances of these types as ivars now that it is convenient to print them.

dzenanz avatar Aug 31 '21 18:08 dzenanz

@ChristianJacobsen contributions are always welcome :100:, and as @dzenanz says, this is still relevant. It would be awesome to finally have all necessary overloads in ITK ! So thanks for considering this :+1:.

Besides what @dzenanz pointed, section C.21 of the ITK SW Guide gives some more guidance about ivar printing in ITK. Essentially we would like to overload the << operator or for all non-trivial types so that we can potentially print any type in ITK with a simple one-liner, e.g.

os << indent << "OurNonTrivialType: " << m_OurNonTrivialType << std::endl

That would allow us to be able to:

  • nicely print such non-trivial types,
  • avoid boiler-plate code (e.g. loops to iterate over the components types) in PrintSelf methods, and
  • print the types in a consistent manner.

PR #512 is related to this. Essentially, all ivars are expected to be printed in a method's PrintSelf method. You will see that this is not always the case: https://open.cdash.org/viewCoverage.php?buildid=7432448

And some other times we may still encounter loops to print some ivars due to a lacking support for the overload.

Some other times ivars are serialized but the lack of appropriate support makes their address to be printed instead of their content (or some more meaningful information), or the itk::print_helper namespace is still missing, etc. e.g. PR #2226 never made it to master, for example, but I had it in my ToDo list to revisit it. So you can also take it if you are willing to.

Booleans have not yet got the corresponding treatment either (cf. this comment), and at times their are directly serialized to their integer values, other times an if/else is used to print the On/Off or True/False strings, etc.

A nice side benefit of printing all ivars is that by doing so at times we identify find some ivars that have not been initialized, which is an undesirable situation, and may lead to a bug. So printing all ivars, hopefully using such useful overloads, is important.

Many of the PRs that have as subject ENH: Improve coverage (...) or a similar text add statements to print the related classes' ivars, so you can get some inspiration from them as well.

Hope this helps. Thanks @ChristianJacobsen .

jhlegarreta avatar Sep 01 '21 13:09 jhlegarreta

@ChristianJacobsen Another good candidates to get this enhancement are these two: https://github.com/InsightSoftwareConsortium/ITK/blob/f81578bbb6242032b611d7e3e4dca3b7557fbbe0/Modules/Core/Common/include/itkNeighborhood.hxx#L170

m_StrideTable and m_OffsetTable.

Or the m_MembershipFunctions vector here: https://github.com/InsightSoftwareConsortium/ITK/blob/047fa6934ac32711d1d7b84fa6eb81c72db01960/Modules/Segmentation/Classifiers/include/itkClassifierBase.hxx#L51

One would call itkPrintSelfObjectMacro(MemberThanCanBeNull) for objects that can be null pointers; however in this case, we have a vector, where each of the values can be so.

Or still, removing the need for the boilerplate code to check for null raw pointers, as in: https://github.com/InsightSoftwareConsortium/ITK/blob/966c9e56f70b7c60ecb05cccb93a3463d4fd5852/Modules/Filtering/Convolution/include/itkConvolutionImageFilterBase.hxx#L118

Thanks.

jhlegarreta avatar Oct 03 '21 22:10 jhlegarreta

Hi @jhlegarreta! Sorry for the late reply. Things at work are just so hectic at the moment that I barely have any spare time left. I will continue to look at this when my schedule clears up. Feel free to reassign responsibility should someone else want to do it instead. Thank you for the tip about itkPrintSelfObjectMacro! 😄

ChristianJacobsen avatar Oct 07 '21 06:10 ChristianJacobsen

@ChristianJacobsen no worries. Take your time. Thanks.

jhlegarreta avatar Oct 07 '21 12:10 jhlegarreta