Skip to content

STYLE: Remove SetFixedParameters... functions from BSplineBaseTransform - #6854

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Remove-protected-SetFixedParameters-functions-BSplineBaseTransform
Sep 8, 2026
Merged

STYLE: Remove SetFixedParameters... functions from BSplineBaseTransform#6854
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Remove-protected-SetFixedParameters-functions-BSplineBaseTransform

Conversation

@N-Dekker

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

Copy link
Copy Markdown
Contributor

Removed the protected SetFixedParametersFromTransformDomainInformation() from BSplineBaseTransform, and moved the code of its implementation into the default-constructor of BSplineDeformableTransform, which was the only place where the function was called.

Removed four protected pure virtual member functions from BSplineBaseTransform:

SetFixedParametersGridSizeFromTransformDomainInformation()
SetFixedParametersGridOriginFromTransformDomainInformation()
SetFixedParametersGridSpacingFromTransformDomainInformation()
SetFixedParametersGridDirectionFromTransformDomainInformation()

@github-actions github-actions Bot added area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) labels Sep 8, 2026
@N-Dekker
N-Dekker marked this pull request as ready for review September 8, 2026 15:57
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This update removes obsolete BSpline fixed-parameter initialization hooks, keeps the specialized deformable-transform initialization path, and cleans up the stale documentation grouping marker. The documentation grouping is now balanced, and no new issues require action.

Confidence Score: 5/5

Safe to merge.

No outstanding findings remain. The previously reported documentation issue was fully addressed: N-Dekker removed the stale terminator, and the remaining documentation group has matching start and end markers.

Reviews (2): Last reviewed commit: "STYLE: Remove SetFixedParameters... func..." | Re-trigger Greptile

Comment thread Modules/Core/Transform/include/itkBSplineTransform.h Outdated
Removed the protected `SetFixedParametersFromTransformDomainInformation()` from
BSplineBaseTransform, and moved the code of its implementation into the
default-constructor of BSplineDeformableTransform, which was the only place
where the function was called.

Removed four protected pure virtual member functions from BSplineBaseTransform:

    SetFixedParametersGridSizeFromTransformDomainInformation()
    SetFixedParametersGridOriginFromTransformDomainInformation()
    SetFixedParametersGridSpacingFromTransformDomainInformation()
    SetFixedParametersGridDirectionFromTransformDomainInformation()

Left them implemented by BSplineDeformableTransform, but removed their dummy
(empty) implementations from BSplineTransform. They were already made empty by
pull request InsightSoftwareConsortium#297
commit 794c82d
"ENH: Refactor BSplineTransform", Bradley Lowekamp, merged on Dec 13, 2018.
@N-Dekker

N-Dekker commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

External subclasses that retain override declarations for the four removed grid-information hooks no longer compile.

BSplineBaseTransform was never meant to be used as base class of external subclasses. It was just meant to be the common base class of ITK's own BSplineTransform and BSplineDeformableTransform. As suggested by its documentation at

* \brief A base class with common elements of BSplineTransform and BSplineDeformableTransform
Which says about BSplineBaseTransform:

A base class with common elements of BSplineTransform and BSplineDeformableTransform

So then I think we shouldn't worry about breaking support for some hypothetical external subclasses.


For the record, BSplineBaseTransform was introduced by commit 937439c, "STYLE: Common base class for BSpline*Transforms", Nov 4, 2011, by Hans Johnson (@hjmjohnson).

@N-Dekker
N-Dekker marked this pull request as draft September 8, 2026 16:37
@N-Dekker
N-Dekker force-pushed the Remove-protected-SetFixedParameters-functions-BSplineBaseTransform branch from ee70a3a to 9953d5f Compare September 8, 2026 16:38
@N-Dekker
N-Dekker marked this pull request as ready for review September 8, 2026 16:41
@hjmjohnson
hjmjohnson merged commit fd3b0b4 into InsightSoftwareConsortium:main Sep 8, 2026
20 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 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