BUG: SAGITAL_PLANE -> SAGITTAL_PLANE, though with legacy support

[Leengit]

Closes #3923.

Replace the misspelling SAGITAL_PLANE with SAGITTAL_PLANE.

Note that both spellings are accepted if not ITK_LEGACY_REMOVE.

However, invocation of std::ostream &operator<<(std::ostream & out, const OctreeEnums::Octree value) with either spelling will output a string with the correct spelling: "itk::OctreeEnums::Octree::SAGITTAL_PLANE"

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)
• Added test (or behavior not changed)
• Updated API documentation (or API not changed)
• Added license to new files (if any)
• Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
• Added ITK examples for all new major features (if any)

[hjmjohnson]

There is a dashboard issue that might be tricky to work around:
https://open.cdash.org/viewBuildError.php?type=1&buildid=10163799

[dzenanz]

Could we fix this via warning suppression mechanism?

[Leengit]

The dashboard is showing the warnings for deprecation when we haven't set ITK_LEGACY_REMOVE... we allow the deprecated spelling and get the warning. Should I remove [[deprecated]] under the theory that the user didn't set ITK_LEGACY_REMOVE and therefore doesn't want the deprecation warnings?

[dzenanz]

Could we fix this via warning suppression mechanism?

I found an example of this:

ITK/.circleci/config.yml

Line 74 in 524df2a

-DCTEST_CUSTOM_WARNING_EXCEPTION:STRING="itkIndex.h:.*warning: array subscript is above array bounds" \

[dzenanz]

remove [[deprecated]]

This is the easiest solution. @blowekamp what do you think?

[blowekamp]

Could we fix this via warning suppression mechanism?

The ITK_FUTURE_DEPRECATED may work here.

Recently in itkAnatomicalOrientation I also had to add an ITK_WRAPPING. That may need to be added to the itkMacro.

ITK/Modules/Core/Common/include/itkAnatomicalOrientation.h

Lines 436 to 445 in 5a6d1e9

	#ifndef ITK_FUTURE_LEGACY_REMOVE
	/** \brief Conversion from Legacy SpatialOrientation
	*
	* @param legacyOrientation
	*/
	# if defined(ITK_LEGACY_REMOVE) && !defined(ITK_LEGACY_SILENT) && !defined(ITK_WRAPPING)
	[[deprecated("Use the AnatomicalOrientation::FromEnum type instead.")]]
	# endif
	AnatomicalOrientation(LegacyOrientationType legacyOrientation);
	#endif

[dzenanz]

added to the itkMacro

If you add it to itkMacro.h, and change the AnatomicalOrientation to show how the new macro is supposed to be used, I am convinced that Lee can use it in this PR.

[blowekamp]

added to the itkMacro

If you add it to itkMacro.h, and change the AnatomicalOrientation to show how the new macro is supposed to be used, I am convinced that Lee can use it in this PR.

@Leengit Please give the macro defined here a try:

ITK/Modules/Core/Common/include/itkMacro.h

Lines 1383 to 1389 in 1090d31

	// ITK_FUTURE_DEPRECATED is only for internal use, within the implementation of ITK. It allows triggering "deprecated"
	// warnings when legacy support is removed, which warn that a specific feature may be removed in the future.
	#if defined(ITK_LEGACY_REMOVE) && !defined(ITK_LEGACY_SILENT)
	# define ITK_FUTURE_DEPRECATED(message) [[deprecated(message)]]
	#else
	# define ITK_FUTURE_DEPRECATED(message)
	#endif

If warning persists with wrapping, then the macro may need to be modified to handle when ITK_WRAPPING is defined.

[Leengit]

I am thinking that the optimal behavior is

1. if ITK_LEGACY_REMOVE IS NOT defined then the misspelling is available. The misspelling should not produce a warning when building 3D Slicer itself, but it would be nice to have a [[deprecated]] warning in code that invokes 3D Slicer.
2. if ITK_LEGACY_REMOVE IS defined then the misspelling is not available. It is a fatal error to use the misspelling.

It looks like use of ITK_FUTURE_DEPRECATED would
3. allow the misspelling even though ITK_LEGACY_REMOVE IS defined, producing a [[deprecated]] warning, but not a fatal error.

So am I hesitant to use ITK_FUTURE_DEPRECATED. Or is my ideal something I should be thinking about differently?

[Leengit]

I just force-pushed changes that I hope have the following behavior.

1. When ITK_LEGACY_REMOVE is set then the misspellings are not defined, and other software that tries to use a misspelling will get a fatal error.
2. When ITK_LEGACY_REMOVE is not set the misspellings are defined in terms of the proper spellings (and never in terms of other objects that are misspelled) and thus 3D Slicer should build without warnings or errors.
3. When ITK_LEGACY_REMOVE is not set the misspellings are marked as [[deprecated]], so other software that uses the misspellings will get a deprecation warning.

[Leengit]

As I discuss above, I was able to achieve a balancing of priorities ... but I acknowledge that there are other ways to balance the priorities. If we should go forward with the current implementation, please approve. Otherwise, let's continue the discussion here. Thanks!

And/or if you think the failure of itkFFTConvolutionImageFilterStreamingTest is somehow related to this PR, please let me know.