Skip to content

Conversation

@N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Jun 17, 2025

Added enumerators to IOComponent that explicitly specify the number of bits of the types they refer to:

UINT8, INT8, UINT16, INT16, UINT32, INT32, UINT64, INT64, FLOAT32, FLOAT64

Tested that MapPixelType<TPixel>::CType maps to the expected enumerator.

INT8 is clearer than CHAR, because the C++ type char may be either signed
or unsigned. (INT8 corresponds with std::int8_t, which is always signed, by
definition.) INT32 and INT64 are clearer than LONG, because long may be
either 32 bits or 64 bits, depending on the platform and the compiler. FLOAT32
is clearer than FLOAT, because on other languages (notably Python), the
standard float type may be 64 bits.

@github-actions github-actions bot added type:Enhancement Improvement of existing methods or implementation 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:IO Issues affecting the IO module labels Jun 17, 2025
@N-Dekker N-Dekker force-pushed the IOComponent-fixed-width-type-enums branch from d952af0 to 2a335fe Compare June 17, 2025 17:59
Added enumerators to `IOComponent` that explicitly specify the number of bits
of the types they refer to:

    UINT8, INT8, UINT16, INT16, UINT32, INT32, UINT64, INT64, FLOAT32, FLOAT64

Tested that `MapPixelType<TPixel>::CType` maps to the expected enumerator.

`INT8` is clearer than `CHAR`, because the C++ type `char` may be either signed
or unsigned. (`INT8` corresponds with `std::int8_t`, which is always signed, by
definition.) `INT32` and `INT64` are clearer than `LONG`, because `long` may be
either 32 bits or 64 bits, depending on the platform and the compiler. `FLOAT32`
is clearer than `FLOAT`, because on other languages (notably Python), the
standard `float` type may be 64 bits.
@N-Dekker N-Dekker force-pushed the IOComponent-fixed-width-type-enums branch from 2a335fe to d2e6801 Compare June 17, 2025 18:02
@N-Dekker N-Dekker marked this pull request as ready for review June 17, 2025 18:05
@hjmjohnson
Copy link
Member

Windows failure is due to Github removing support for windows-2019

@hjmjohnson hjmjohnson merged commit ab87323 into InsightSoftwareConsortium:master Jun 18, 2025
15 of 16 checks passed
@seanm
Copy link
Contributor

seanm commented Jun 19, 2025

This change introduced -Wduplicate-enum warnings into ITK, and there were none before.

Any objection to wrapping the enum in a pragma push/pop disabling that warning around that enum?

@dzenanz
Copy link
Member

dzenanz commented Jun 19, 2025

None - quite the contrary. I noticed 199 (maximum reported number?) of new warnings on your dashboard machines. It would be good if someone fixed it :D

@seanm
Copy link
Contributor

seanm commented Jun 19, 2025

None - quite the contrary. I noticed 199 (maximum reported number?) of new warnings on your dashboard machines. It would be good if someone fixed it :D

#5422

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:IO Issues affecting the IO module type:Enhancement Improvement of existing methods or implementation 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.

4 participants