Skip to content

COMP: Use future recommended code for remote modules with ITKv5.4.4 #5183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

hjmjohnson
Copy link
Member

@hjmjohnson hjmjohnson commented Jan 25, 2025

These selected remote modules are chosen to support
the building of heavily used projects that depend
on ITK.

Slicer (https://github.com/Slicer/Slicer)
BRAINSTools (https://github.com/BRAINSia/BRAINSTools)
ANTs (https://github.com/ANTsX/ANTs)

needs of remote modules for ITK.

When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

Identifies code changes that will eventually be needed in
future version (i.e. ITKv6 and ITKv7) once deprecated
code is removed.

By updating the following remote modules to be backward and
future compatible with the recommended coding standards, we
can more easily address code updates for supporting the
community.

Pull requests that this one depends upon:

PR Checklist

@hjmjohnson hjmjohnson added type:Compiler Compiler support or related warnings type:Style Style changes: no logic impact (indentation, comments, naming) labels Jan 25, 2025
@hjmjohnson hjmjohnson added this to the ITK 5.4.2 milestone Jan 25, 2025
@hjmjohnson hjmjohnson requested review from thewtex and dzenanz January 25, 2025 18:14
@hjmjohnson hjmjohnson self-assigned this Jan 25, 2025
@hjmjohnson hjmjohnson modified the milestones: ITK 5.4.2, ITK 5.4.3 Jan 25, 2025
@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Video Issues affecting the Video module area:Remotes Issues affecting the Remote module area:Documentation Issues affecting the Documentation module and removed type:Compiler Compiler support or related warnings type:Style Style changes: no logic impact (indentation, comments, naming) labels Jan 25, 2025
@hjmjohnson hjmjohnson marked this pull request as draft January 25, 2025 20:22
@hjmjohnson hjmjohnson changed the title COMP: Use future recommended code for remote modules COMP: Use future recommended code for remote modules with ITKv5.4.3 Jan 26, 2025
@dzenanz
Copy link
Member

dzenanz commented Jan 27, 2025

Approach looks good. Most of the work will be in "greening" the remote modules.

seanm and others added 13 commits January 31, 2025 16:35
Fix incorrect `BibTeX` citation keys.

Fixes:
```
.../ITK/Modules/Filtering/ImageGrid/include/itkBSplineResampleImageFilterBase.h:47:
 warning: \cite command to 'unser199' does not have an associated number

.../ITK/Modules/Core/Common/include/itkGaussianDerivativeOperator.h:80:
 warning: \cite command to 'lindeberg1999' does not have an associated number

.../ITK/Modules/Filtering/Convolution/include/itkMaskedFFTNormalizedCorrelationImageFilter.h:128:
 warning: \cite command to 'padfield' does not have an associated number
```

Reported by: albert-github <[email protected]>

Co-authored-by: albert-github <[email protected]>
Use `ITK_TRY_EXPECT_NO_EXCEPTION` macro in `ImageCompose` tests:
- Use `ITK_TRY_EXPECT_NO_EXCEPTION` macro when updating filters in lieu
  of `try/catch` blocks for the sake of readability and compactness, and
  to save typing/avoid boilerplate code.
- Only place with the macro the code that might raise exceptions.
Conform to ITK style guidelines in test ending message across the
`ImageCompose` module test files: add it where missing and use the same
message consistently.

Take advantage of the commit to remove unnecessary comments about
objects being destroyed when exiting the test main method.
Conform to ITK style guidelines in test argument check message across
the `ImageCompose` module test files: add the missing argument message
where missing and use the same message consistently.
Perform first the test input argument check across the `ImageCompose`
module test files.

Take advantage of the commit to add the missing argument message where
missing and use the same message consistently.
Remove unnecessary/uninformative comment block in `ImageCompose` module
test.
Do not capitalize variable first letter: adhere to ITK style guidelines.
…scImageComposeTestStyleEnh

STYLE: Miscellaneous `ImageCompose` test style enhancements
This is a follow-up to bbffe4f.
Moved the member function definitions of member functions that have an empty
member function definition from their "itk*.hxx" file to the corresponding
"itk*.h" file, and into their class definition.

Found by regular expression `^..[^:,].+\)\r\n{}` in "itk*.hxx" files.

- Follow-up to pull request InsightSoftwareConsortium#5198
commit b94770d
"STYLE: Move empty member functions with empty param list from .hxx to .h"
Closes InsightSoftwareConsortium#5179. To address warnings of the style:

...ITK/Modules/Core/Common/include/itkImageBoundaryCondition.h:123: warning: argument 'outputRequestedRegion' of command @param is not found in the argument list of itk::ImageBoundaryCondition< TInputImage, TOutputImage >::GetInputRequestedRegion(const RegionType &inputLargestPossibleRegion, const RegionType &) const
dg0yt and others added 22 commits May 14, 2025 08:15
ITK_MSVC_STATIC_CRT was exported from installed ITKConfig.cmake, but
it wasn't available when building external modules (e.g. RTK) together
with ITK. This caused linking errors for tools.
Modernized the code. Also improved the style, by removing `static_cast<float>`s.
Replaced regular expression `auto[ ]+(\w+) = ([\w:]+){ ([^}]*) };` with
`$2 $1{ $3 };`

Aims to make the coding style more uniform.
Replaced regular expression `auto[ ]+(\w+) = ([\w:]+){};` with `$2 $1{};`

Aims to make the coding style more uniform.
COMP: Provide ITK_MSVC_STATIC_CRT to in-tree build
…ce-rand-with-mt19937

STYLE: Replace `rand()` calls with `std::mt19937` in ImageAdaptor test
…e-auto-from-local-variable-initializations

Replace `auto var = T{...}` with `T var{...}`
The original division of `rand()` by `32768.0` was probably meant to yield a
number between `[0, 1>`. However, `rand()` returns an integer between
`[​0​, RAND_MAX]`, and `RAND_MAX` may not be `32767`. In practice, when using
GCC, `RAND_MAX` may be as large as `2147483647.

The "magic number" `32768.0` was introduced on 12 Jan 2002 already, with commit
074c74f.
…ce-Initialize-with-SetSeed

FUTURE REMOVE `MersenneTwisterRandomVariateGenerator::Initialize`
…andom-number-RGBGibbsPriorFilter

BUG: Fix random number in `RGBGibbsPriorFilter::GibbsTotalEnergy`
Dicomfile with signed int data (pixel rep=1) can be decoded as uint with the
correct intercept. The assertion blocking this behavior has been changed to a
more meaningful check.

Closes InsightSoftwareConsortium#5290
…kOptImageToImageMetricsTest04

BUG: Make itkOptImageToImageMetricsTest04 not to fail on single-CPU s…
…es_reader_print_filenames

ENH: Add printing of FileNames to ImageSeriesReader
…valgrind_supression

ENH: Add valgrind suppression file for Ubuntu 22.04 LTS
It is recommended to use C++11's random number generation facilities (like
`std::mt19937`) to replace `rand()`, according to
https://en.cppreference.com/w/cpp/numeric/random/rand
Clarify further remote module major version bumps with ITK minor
releases.
@hjmjohnson hjmjohnson force-pushed the new-tag-v5.4.3-remote-slicer-support branch from e2ca9d3 to 4071d54 Compare May 27, 2025 22:23
@github-actions github-actions bot removed type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Video Issues affecting the Video module area:Remotes Issues affecting the Remote module area:Documentation Issues affecting the Documentation module labels May 27, 2025
When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ -  itkTypeMacro
│ +  itkOverrideGetNameOfClassMacro
@hjmjohnson hjmjohnson closed this May 27, 2025
@hjmjohnson hjmjohnson force-pushed the new-tag-v5.4.3-remote-slicer-support branch from 4071d54 to 19d7218 Compare May 27, 2025 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.