Deprecate `CoordRepType` and replace it with `CoordinateType` #4997

N-Dekker · 2024-11-27T17:11:03Z

"CoordinateType" appears more readable than "CoordRepType".

Using Notepad++, Replace in Files, doing:

Find what: ( [ ]+)using CoordRepType = (\w+);
Replace with: $1using CoordinateType = $2;\r\n$1using CoordRepType = CoordinateType;
Filters: itk*.h
Directory: D:\src\ITK\Modules
[v] Match case
(*) Regular expression

Follow-up to pull request STYLE: Rename TCoordRep template parameters to TCoordinate #4995

dzenanz

I would prefer if you introduced deprecation for CoordRepType in this PR. Pick legacy or future legacy.

N-Dekker · 2024-11-27T20:44:23Z

Modules/Core/Common/include/itkBoundingBox.h

-  using CoordRepType = CoordinateType;
+#ifndef ITK_FUTURE_REMOVE_LEGACY
+  using CoordRepType [[deprecated("ITK 6 discourages using `CoordRepType`. Please use `CoordinateType` instead!")]] =
+    CoordinateType;
+#endif


I'm not sure but I'm afraid that some users might get annoyed by this deprecated-warning. Especially those users who have intentionally enabled legacy support. What do you think?

I'm thinking, maybe we should have a FUTURE_DEPRECATED macro, roughly as follows:

#ifdef ITK_LEGACY_REMOVE #define FUTURE_DEPRECATED [[deprecated]] #else #define FUTURE_DEPRECATED #endif

To be used as follows:

using CoordRepType FUTURE_DEPRECATED = CoordinateType;

So that the warning only appears when users enable LEGACY_REMOVE. What do you think?

There are compiler flags to control warnings.

I now notice: is the correct ifdef ITK_FUTURE_LEGACY_REMOVE? You used ITK_FUTURE_REMOVE_LEGACY.

That FUTURE_DEPRECATED macro sounds good to me.

On the other hand, I just hope that this PR can be processed quickly, before Hans (@hjmjohnson) submits another mega-PR 😸 His PR #4996 already caused some conflicts locally, when I was preparing this PR.

"CoordinateType" appears more readable than "CoordRepType". Using Notepad++, Replace in Files, doing: Find what: ( [ ]+)using CoordRepType = (\w+); Replace with: $1using CoordinateType = $2;\r\n$1using CoordRepType = CoordinateType; Filters: itk*.h Directory: D:\src\ITK\Modules [v] Match case (*) Regular expression Manually excluded "itkQuadEdgeMeshEulerOperatorsTestHelper.h", as it only declares `CoordRepType` locally inside a function. - Follow-up to pull request InsightSoftwareConsortium#4995 commit c1d9ff5 "STYLE: Rename `TCoordRep` template parameters to `TCoordinate`"

Did Replace in Files on "itk*Test*.cxx" and "itk*Test*.h".

Did Notepad++ Replace in Files in ITK/Modules, on "itk*.cxx", "itk*.hxx", and "itk*.h". Skipped the `using CoordRepType = CoordinateType` declarations.

Deprecated using `CoordRepType`, as was suggested by Dženan Zukić.

jhlegarreta · 2024-11-30T14:30:30Z

I assume this
https://open.cdash.org/viewBuildError.php?onlydeltap&buildid=10058584

is related to this PR.

N-Dekker · 2024-12-02T10:45:04Z

@jhlegarreta Thanks Jon. I see:

/Users/builder/externalExamples/SpatialObjects/MeshSpatialObject.cxx:66:13: error: no type named 'CoordRepType' in 'itk::Mesh<float, 3, itk::DefaultDynamicMeshTraits<float, 3>>'

Do you have a clue why? Does it have ITK_FUTURE_LEGACY_REMOVE defined?

'CoordRepType' should only be removed when ITK_FUTURE_LEGACY_REMOVE is defined:

https://github.com/N-Dekker/ITK/blob/c753171d0b6461e947f2d9ec8fe808392e4bfd01/Modules/Core/Mesh/include/itkMesh.h#L172-L174

jhlegarreta · 2024-12-02T13:44:46Z

Haven't had a look/do not have time to investigate now, but my first thought was whether the Examples were included in your set of changes.

dzenanz · 2024-12-02T14:27:01Z

@seanm might provide more info about the build configuration.

N-Dekker · 2024-12-02T14:35:01Z

Sorry I overlooked the Examples when I did this PR. I can have another look in a few hours from now.

Addressed a compile error at Mac12.x-AppleClang-dbg-Universal saying: > MeshSpatialObject.cxx:66:13: error: no type named 'CoordRepType' in > 'itk::Mesh<float, 3, itk::DefaultDynamicMeshTraits<float, 3>>' Reported by Jon Haitz Legarreta Gorroño. - Follow-up to pull request InsightSoftwareConsortium#4997 commit 15d189f "STYLE: Replace CoordRepType with CoordinateType in ITK implementation"

Addressed a compile error at Mac12.x-AppleClang-dbg-Universal saying: > MeshSpatialObject.cxx:66:13: error: no type named 'CoordRepType' in > 'itk::Mesh<float, 3, itk::DefaultDynamicMeshTraits<float, 3>>' Reported by Jon Haitz Legarreta Gorroño. - Follow-up to pull request #4997 commit 15d189f "STYLE: Replace CoordRepType with CoordinateType in ITK implementation"

Follow-up to pull request InsightSoftwareConsortium#4997 commit 5ea1a9a "ENH: Add nested CoordinateType aliases as alternative to CoordRepType"

Follow-up to pull request InsightSoftwareConsortium#4997 commit 324eaf1 "STYLE: Replace CoordRepType with CoordinateType in tests"

Follow-up to pull request InsightSoftwareConsortium#4997 commit 15d189f "STYLE: Replace CoordRepType with CoordinateType in ITK implementation"

Follow-up to pull request InsightSoftwareConsortium#4997 commit e667fa3 "ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType"

Follow-up to pull request InsightSoftwareConsortium#4997 commit 15d189f "STYLE: Replace CoordRepType with CoordinateType in ITK implementation"

Follow-up to pull request InsightSoftwareConsortium#4997 commit e667fa3 "ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType"

Follow-up to pull request #4997 commit 5ea1a9a "ENH: Add nested CoordinateType aliases as alternative to CoordRepType"

Follow-up to pull request #4997 commit 324eaf1 "STYLE: Replace CoordRepType with CoordinateType in tests"

Follow-up to pull request #4997 commit 15d189f "STYLE: Replace CoordRepType with CoordinateType in ITK implementation"

Follow-up to pull request #4997 commit e667fa3 "ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType"

Redid pull request InsightSoftwareConsortium#4997 commit 5ea1a9a on release-5.4

Redid pull request #4997 commit 5ea1a9a on release-5.4

Addressed the warnings at https://open.cdash.org/viewBuildError.php?type=1&buildid=10063093 reported by Dženan Zukić at InsightSoftwareConsortium/ITK#5006 (comment) Following ITK pull request InsightSoftwareConsortium/ITK#4997 commit InsightSoftwareConsortium/ITK@e667fa3

Added note about `CoordRepType` being replaced with `CoordinateType`. Follow-up to: - pull request InsightSoftwareConsortium#5006 commit a68b8f6 "ENH: FUTURE REMOVE `...CoordRepType` in favor of `...CoordinateType`" - pull request InsightSoftwareConsortium#4997 commit e667fa3 "ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType"

Added note about `CoordRepType` being replaced with `CoordinateType`. Follow-up to: - pull request #5006 commit a68b8f6 "ENH: FUTURE REMOVE `...CoordRepType` in favor of `...CoordinateType`" - pull request #4997 commit e667fa3 "ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType"

Following ITK pull request InsightSoftwareConsortium/ITK#4997 "Deprecate `CoordRepType` and replace it with `CoordinateType`"

dzenanz reviewed Nov 27, 2024

View reviewed changes

N-Dekker force-pushed the Add-CoordinateType branch from bd4620d to 8ac75d5 Compare November 27, 2024 20:02

github-actions bot added area:IO Issues affecting the IO module area:Video Issues affecting the Video module area:Numerics Issues affecting the Numerics module labels Nov 27, 2024

N-Dekker changed the title ~~ENH: Added nested CoordinateType aliases as alternative to CoordRepType~~ Deprecate CoordRepType and replace it with CoordinateType Nov 27, 2024

dzenanz approved these changes Nov 27, 2024

View reviewed changes

N-Dekker commented Nov 27, 2024

View reviewed changes

N-Dekker force-pushed the Add-CoordinateType branch from 8ac75d5 to 836d2a6 Compare November 27, 2024 21:13

github-actions bot removed the type:Enhancement Improvement of existing methods or implementation label Nov 27, 2024

N-Dekker marked this pull request as ready for review November 27, 2024 21:21

N-Dekker added 4 commits November 28, 2024 12:51

STYLE: Replace CoordRepType with CoordinateType in tests

ea7d05c

Did Replace in Files on "itk*Test*.cxx" and "itk*Test*.h".

STYLE: Replace CoordRepType with CoordinateType in ITK implementation

9dd3d43

Did Notepad++ Replace in Files in ITK/Modules, on "itk*.cxx", "itk*.hxx", and "itk*.h". Skipped the `using CoordRepType = CoordinateType` declarations.

ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType

c753171

Deprecated using `CoordRepType`, as was suggested by Dženan Zukić.

N-Dekker force-pushed the Add-CoordinateType branch from 836d2a6 to c753171 Compare November 28, 2024 12:08

dzenanz approved these changes Nov 29, 2024

View reviewed changes

dzenanz merged commit e667fa3 into InsightSoftwareConsortium:master Nov 29, 2024
17 checks passed

N-Dekker mentioned this pull request Dec 2, 2024

COMP: Replace "CoordRep" with "Coordinate" in Examples #5004

Merged

N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Dec 3, 2024

STYLE: Replace ...CoordRepType with ...CoordinateType in tests

f7a9236

Follow-up to pull request InsightSoftwareConsortium#4997 commit 324eaf1 "STYLE: Replace CoordRepType with CoordinateType in tests"

N-Dekker mentioned this pull request Dec 3, 2024

Deprecate ...CoordRepType and replace them with ...CoordinateType #5006

Merged

hjmjohnson pushed a commit that referenced this pull request Dec 4, 2024

ENH: Add ...CoordinateType aliases as alternative to ...CoordRepType

fbe9b04

Follow-up to pull request #4997 commit 5ea1a9a "ENH: Add nested CoordinateType aliases as alternative to CoordRepType"

hjmjohnson pushed a commit that referenced this pull request Dec 4, 2024

STYLE: Replace ...CoordRepType with ...CoordinateType in tests

0522246

Follow-up to pull request #4997 commit 324eaf1 "STYLE: Replace CoordRepType with CoordinateType in tests"

hjmjohnson pushed a commit that referenced this pull request Dec 4, 2024

STYLE: Replace ...CoordRepType with ...CoordinateType within ITK

a0555df

Follow-up to pull request #4997 commit 15d189f "STYLE: Replace CoordRepType with CoordinateType in ITK implementation"

hjmjohnson pushed a commit that referenced this pull request Dec 4, 2024

ENH: FUTURE REMOVE ...CoordRepType in favor of ...CoordinateType

a68b8f6

Follow-up to pull request #4997 commit e667fa3 "ENH: Deprecate (FUTURE REMOVE) CoordRepType in favor of CoordinateType"

N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Dec 4, 2024

ENH: Add nested CoordinateType aliases as alternative to CoordRepType

9476c4c

Redid pull request InsightSoftwareConsortium#4997 commit 5ea1a9a on release-5.4

hjmjohnson pushed a commit that referenced this pull request Dec 5, 2024

ENH: Add nested CoordinateType aliases as alternative to CoordRepType

bdfd0c2

Redid pull request #4997 commit 5ea1a9a on release-5.4

N-Dekker mentioned this pull request Dec 6, 2024

COMP: Replace CoordRepType with CoordinateType InsightSoftwareConsortium/ITKMontage#234

Draft

N-Dekker mentioned this pull request Dec 16, 2024

DOC: Note migration guide replacing CoordinateType with CoordRepType #5076

Merged

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jan 8, 2025

STYLE: COMP: Replace "CoordRep" with "Coordinate"

0b692f4

Following ITK pull request InsightSoftwareConsortium/ITK#4997 "Deprecate `CoordRepType` and replace it with `CoordinateType`"

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jan 8, 2025

STYLE: Replace "CoordRep" with "Coordinate"

24c54ee

Following ITK pull request InsightSoftwareConsortium/ITK#4997 "Deprecate `CoordRepType` and replace it with `CoordinateType`"

N-Dekker mentioned this pull request Jan 9, 2025

Replace "CoordRep" with "Coordinate" SuperElastix/elastix#1280

Merged

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jan 9, 2025

STYLE: Replace "CoordRep" with "Coordinate"

fff7b1b

Following ITK pull request InsightSoftwareConsortium/ITK#4997 "Deprecate `CoordRepType` and replace it with `CoordinateType`"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate `CoordRepType` and replace it with `CoordinateType` #4997

Deprecate `CoordRepType` and replace it with `CoordinateType` #4997

N-Dekker commented Nov 27, 2024

dzenanz left a comment

N-Dekker Nov 27, 2024

N-Dekker Nov 27, 2024

dzenanz Nov 27, 2024

dzenanz Nov 27, 2024

N-Dekker Nov 27, 2024

jhlegarreta commented Nov 30, 2024

N-Dekker commented Dec 2, 2024

jhlegarreta commented Dec 2, 2024

dzenanz commented Dec 2, 2024

N-Dekker commented Dec 2, 2024

Deprecate CoordRepType and replace it with CoordinateType #4997

Deprecate CoordRepType and replace it with CoordinateType #4997

Conversation

N-Dekker commented Nov 27, 2024

dzenanz left a comment

Choose a reason for hiding this comment

N-Dekker Nov 27, 2024

Choose a reason for hiding this comment

N-Dekker Nov 27, 2024

Choose a reason for hiding this comment

dzenanz Nov 27, 2024

Choose a reason for hiding this comment

dzenanz Nov 27, 2024

Choose a reason for hiding this comment

N-Dekker Nov 27, 2024

Choose a reason for hiding this comment

jhlegarreta commented Nov 30, 2024

N-Dekker commented Dec 2, 2024

jhlegarreta commented Dec 2, 2024

dzenanz commented Dec 2, 2024

N-Dekker commented Dec 2, 2024

Deprecate `CoordRepType` and replace it with `CoordinateType` #4997

Deprecate `CoordRepType` and replace it with `CoordinateType` #4997