Doxygen docs missing for subsequent macros

[dzenanz]

Docstrings are missing for macros after the first.

Description

This piece of code:
https://github.com/InsightSoftwareConsortium/ITK/blob/v5.4rc01/Modules/Segmentation/SuperPixel/include/itkSLICImageFilter.h#L143-L152
produces docstring for SetEnforceConnectivity, but not for GetEnforceConnectivity nor EnforceConnectivityOn. Entry for EnforceConnectivityOff is missing entirely from the doc page:
https://itk.org/Doxygen/html/classitk_1_1SLICImageFilter.html

Expected information

Same docstring appears for all of the methods mentioned above.

[albert-github]

We have here 2 problems:

Entry EnforceConnectivityOff not shown at all
This has been identified in #5193 and has been fixed by means of #5195

No group documentation
This should be automatically done by means of the INPUT_FILTER: ITK/Utilities/Doxygen/itkgroup.py (previously ITK/Utilities/Doxygen/itkgroup.pl) as, as far as I understand, the clang format does not like some kind of empty lines / usage of @{ ... @} / \{ ... \}. When running the INPUT_FILTER over the file ITK/Modules/Segmentation/SuperPixel/include/itkSLICImageFilter.h the filter doesn't change / add anything (neither with the perl nor python script).
So this explains the missing documentation.

Looking at the mentioned part it looks like:

  /** \brief Post processing step to enforce superpixel morphology.
   *
   * Enable an additional computation which ensures all label pixels of
   * the same value are spatially connected. Disconnected labeled components are
   * assigned a new value if of sufficient size, or are relabeled to
   * the previously encountered value if small.
   */
  itkSetMacro(EnforceConnectivity, bool);
  itkGetMacro(EnforceConnectivity, bool);
  itkBooleanMacro(EnforceConnectivity);

In my opinion:
It will be quite hard to fix the INPUT_FILTER script (needs quite a bit of knowledge about the C++ language and the nesting levels etc.).
It would be better to explicitly add the grouping commands at the relevant places and abandon the INPUT_FILTER. This can be either in the for like @{ ... @} or \{ ... \} so:

  /** \brief Post processing step to enforce superpixel morphology.
   *
   * Enable an additional computation which ensures all label pixels of
   * the same value are spatially connected. Disconnected labeled components are
   * assigned a new value if of sufficient size, or are relabeled to
   * the previously encountered value if small.
   * \{
   */
  itkSetMacro(EnforceConnectivity, bool);
  itkGetMacro(EnforceConnectivity, bool);
  itkBooleanMacro(EnforceConnectivity);
  /** \} */

or by means of an ALIASES (when clang-format doesn't like the @{ ... @} nor \{ ... \} part:

ALIASES += itkGroupStart=@{
ALIASES += itkGroupEnd=@}

resulting in code:

  /** \brief Post processing step to enforce superpixel morphology.
   *
   * Enable an additional computation which ensures all label pixels of
   * the same value are spatially connected. Disconnected labeled components are
   * assigned a new value if of sufficient size, or are relabeled to
   * the previously encountered value if small.
   * \itkGroupStart
   */
  itkSetMacro(EnforceConnectivity, bool);
  itkGetMacro(EnforceConnectivity, bool);
  itkBooleanMacro(EnforceConnectivity);
  /** \itkGroupEnd */

(I personally prefer the explicit @{ ... @} / \{ ... \} version).

Note: I think that running doxygen with -d preprocessor once with and once without the filter might help to identify where currently @{ ... @} is inserted so these files can be inspected (will be quite a bit of work though).

[albert-github]

By means of the doxygen comment pair:

/** @ITKStartGrouping */

and

/** @ITKEndGrouping */

this now can be accomplished quite easily, though I think the documentation texts should be checked as well.
Furthermore there might be more files where this problem occurs.

Edit
Examples are e.g.

• Modules/Core/TestKernel/include/itkPipelineMonitorImageFilter.h
• Modules/Core/TestKernel/include/itkTestingComparisonImageFilter.h

Another example is:

• Modules/Filtering/ImageGrid/include/itkChangeInformationImageFilter.h

but here there is an explicit empty line after the comment block so here it might be on purpose so that the "old" script wouldn't kick in (further on in the file there is block where the grouping is done).

[dzenanz]

I don't think the absence of docstrings was intentional. @blowekamp should be able to authoritatively confirm it.

Is there a way to get doxygen to emit a warning (with e.g. some special flag) if a public method is missing a docstring? That would allow us to identify all similar instances.

[albert-github]

Yes I think there are some possibilities for:

Is there a way to get doxygen to emit a warning (with e.g. some special flag) if a public method is missing a docstring? That would allow us to identify all similar instances.

though this possibly will also result in some false positives (warnings about not documented entities etc.). Currently the setting is EXTRACT_ALL=YES and this will suppress quite a few warnings, setting this to NO will give the warnings, but also won't show the functions in the documentation.

I did a quick test and got over 33000 warnings: warnings.log
of which there are over 7200 wit "(function)".
Probably some are not what is required (like having itk::Testing::ComparisonImageFilter ?).

[dzenanz]

7207 is quite a long list. Narrowing this down further, searching warnings.log for Off() (function) yields 102 matches, meaning 100 or so bool setters are missing docstrings (including e.g. EnforceConnectivity). This is a good starting point for fixes.

[blowekamp]

@blowekamp should be able to authoritatively confirm it.

It sounds like I wrote to to apply to all three methods describing the variable's purpose and not specific to set or get.

[albert-github]

@dzenanz
Indeed very long lists but by means of some intelligent filtering I think it will indeed be possible to get some good starting points. The texts with the different items have to be checked by an expert who also wil be able to add the

/** @ITKStartGrouping *

and

/** \itkGroupEnd */