Skip to content

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Jul 11, 2025

Replaced itk::Image<char with itk::Image<signed char, in all "itk*Test*.cxx" source files. Removed pixel type alias InputDataType = char from itkAntiAliasBinaryImageFilterTest, and directly used signed char instead.

The use of char as TPixel argument in tests appears to triggers test failures, when compiling with an unsigned default char type (GCC option -funsigned-char or MSVC option /J), for example:

[ RUN      ] FrequencyIterators.Even2D
Unequal images, with 29 unequal pixels
test/itkFrequencyIteratorsGTest.cxx:260: Failure
Value of: fullAndHermitian
Actual: false
Expected: true

It appears clearer to explicitly specify that the pixel type should be signed, by using signed char instead.


Replaced `itk::Image<char` with `itk::Image<signed char`, in all "itk*Test*.cxx"
source files. Removed pixel type alias `InputDataType = char` from
itkAntiAliasBinaryImageFilterTest, and directly used `signed char` instead.

The use of `char` as `TPixel` argument in tests appears to triggers test
failures, when compiling with an unsigned default `char` type (GCC option
`-funsigned-char` or MSVC option `/J`), for example:

    [ RUN      ] FrequencyIterators.Even2D
    Unequal images, with 29 unequal pixels
    test/itkFrequencyIteratorsGTest.cxx:260: Failure
    Value of: fullAndHermitian
    Actual: false
    Expected: true

It appears clearer to explicitly specify that the pixel type should be signed,
by using `signed char` instead.
@github-actions github-actions bot added area:Python wrapping Python bindings for a class type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) labels Jul 11, 2025
@N-Dekker N-Dekker marked this pull request as ready for review July 11, 2025 14:50
@N-Dekker N-Dekker requested a review from blowekamp July 11, 2025 14:50
Copy link
Member

@blowekamp blowekamp left a comment

Choose a reason for hiding this comment

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

Looks good. With the IO changes, the IO related tests should clearly be signed char. In the level sets the char images are used for mask/label, in this case I am not sure the sign-ness was intended, but I don' think it matters.

@hjmjohnson hjmjohnson merged commit 5796e06 into InsightSoftwareConsortium:main Jul 11, 2025
19 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants