search

InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

STYLE: Remove legacy content from "itkMultiThreader.h" #4693

Closed N-Dekker closed 1 week ago

N-Dekker commented 1 month ago

"itkMultiThreader.h" was deprecated already from ITK 5.0.

dzenanz commented 1 month ago

@thewtex Is the next version number 5.5, or 6.0?

N-Dekker commented 1 month ago

Of course, this specific code change isn't very important to me. I'm just wondering now what we should do about all the "legacy support code" in general, now that 5.4 is tagged.

Clearly it's OK now to mark things "future legacy":

As well as to move things from "future legacy" to "legacy only":

So then it seems to make sense to me to also drop old "legacy only" things. But again, I'm just wondering 🤷

dzenanz commented 1 month ago

drop old "legacy only" things

This should be done consistently (for all legacy things), once we declare the begin of 6.0 development.

dzenanz commented 1 month ago

Something akin to this: 01580b37063ab3eeaa60a9f18068844b4d40e9f5.

N-Dekker commented 1 month ago

Thanks @dzenanz Does that mean that all legacy things are treated the same way? Being: let them be there until ITK 6, and then drop them all? (I can imagine that some legacy things are more bothering than others. For example, I would find it a pity if legacy support of #4691 cannot yet be dropped with ITK 6)

dzenanz commented 1 month ago

With a new major version start, we generally remove all the code which is currently legacy, and convert all future legacy into current legacy. Which is what you already did for a few of your favorites 😄

blowekamp commented 1 month ago

Are we going to have some kinds of "ITKv5_COMPATIBILITY" mode?

dzenanz commented 1 month ago

Would we need anything beyond legacy enabled?

seanm commented 1 month ago

Isn't "STYLE" inappropriate? This is a breaking change, not something merely stylistic.

thewtex commented 1 month ago

The next feature release is ITK 6, and it is in progress.

Removing legacy content should be ENH instead of STYLE as @seanm mentioned.

While the process is generally to eventually drop legacy code, we do not need to automatically or always remove "legacy" and update "future legacy" to "legacy". This can be handled on a case-by-case basis. If the code is still widely used, there is not a clear, straightforward path to migration, and the code is not causing issues, it can be kept.

If deprecated code is removed, an entry should be created in an ITK 6 migration guide here: https://github.com/InsightSoftwareConsortium/ITK/tree/master/Documentation/docs/migration_guides

thewtex commented 1 month ago

Clarifications and initialization started in #4696 #4695

dzenanz commented 2 weeks ago

Time to close this PR and take out the deprecation removal axe? 😄

dzenanz commented 1 week ago

Closing in favor of #4729.