Skip to content

STYLE: Remove int from VectorContainer in comments IsotropicWavelets - #6825

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Remove-int-from-VectorContainer-IsotropicWavelets
Sep 2, 2026
Merged

STYLE: Remove int from VectorContainer in comments IsotropicWavelets#6825
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Remove-int-from-VectorContainer-IsotropicWavelets

Conversation

@N-Dekker

@N-Dekker N-Dekker commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Removed the int template argument from the VectorContainer type aliases in comments in "IsotropicWavelets". The default index type (Identifier Type) is usually preferable for VectorContainer. The default index type does not need to be specified, when using VectorContainer in C++.

Removed the `int` template argument from the VectorContainer type aliases in
comments in "IsotropicWavelets". The default index type (Identifier Type) is
usually preferable for VectorContainer. The default index type does not need
to be specified, when using VectorContainer in C++.

Following pull request InsightSoftwareConsortium#4879
commit c103044
"STYLE: Remove VectorContainer Identifier template argument from comments"
@github-actions github-actions Bot added area:Filtering Issues affecting the Filtering module type:Style Style changes: no logic impact (indentation, comments, naming) labels Sep 1, 2026
@N-Dekker
N-Dekker marked this pull request as ready for review September 1, 2026 13:12
@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change updates the IsotropicWavelets header examples to use the one-argument public itk::VectorContainer alias.

The reported issue was disproved by compiling the exact documented spelling, itk::VectorContainer<OutputImagePointer>, against the checked-out ITK headers. The compilation succeeded and compile-time assertions confirmed that the alias selects itk::SizeValueType as its identifier type and OutputImagePointer as its element type. The two-argument requirement applies to the underlying implementation type, not the public alias.

Confidence Score: 5/5

Safe to merge: the updated header examples use a valid public ITK type alias.

The exact alias in the modified comment was compiled successfully, with static assertions verifying its resolved identifier and element types. No publishable defects remain.

Files Needing Attention: None. The checked example in Modules/Filtering/IsotropicWavelets/include/itkRieszFrequencyFilterBankGenerator.h is valid, and the matching one-argument examples in the other changed IsotropicWavelets headers use the same supported alias form.

T-Rex T-Rex Logs

What T-Rex did

  • I compiled the exact documented public spelling, typename itk::VectorContainer<OutputImagePointer>, and the compilation exited successfully with static assertions confirming the alias resolves to itk::SizeValueType for the identifier and OutputImagePointer for the element.
  • I compiled the underlying itk::detail::VectorContainer<OutputImagePointer> control type, and the compilation failed because the implementation type requires two template parameters, confirming that the public alias requirement does not apply to the underlying implementation.
  • I inspected the changed IsotropicWavelets header comments and confirmed that the same valid public alias form is used in each modified IsotropicWavelets header.
  • In a before/after examination, the underlying two-parameter control previously failed to compile, while the exact public alias compiled successfully after the change, and the exact-alias source and header edits were verified by the corresponding logs and diffs.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "STYLE: Remove int from VectorContainer i..." | Re-trigger Greptile

@dzenanz dzenanz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does the code work if you uncomment these declarations and comment out the std::vector?

@N-Dekker

N-Dekker commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the approval!

Does the code work if you uncomment these declarations and comment out the std::vector?

Well, it does, because the IsotropicWavelets module is switched off by default. 😸

However, if you do switch it on, it appears that the code assumes that these containers are default-constructible. std::vector is default-constructible, but itk::VectorContainer is not. So uncommenting these declarations and commenting out std::vector won't compile out-of-the-box.

On Oct 3, 2017, Pablo (@phcerdan) wrote about this, in commit 462d5fc:

Preparation for changing std::vector<ImagePointer> to itk::VectorContainer<int, ImagePointer>.
It can be used in python, and it allow pipelines updates.

For me personally, std::vector<ImagePointer> would be just fine. But if we would ever want to replace it with itk::VectorContainer, let's please just use the default index type for itk::VectorContainer! 🙏

@hjmjohnson
hjmjohnson merged commit 70755a3 into InsightSoftwareConsortium:main Sep 2, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module type:Style Style changes: no logic impact (indentation, comments, naming)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants