Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Mark LabelGeometryImageFilter as deprecated #4630

Merged

Conversation

blowekamp
Copy link
Member

The LabelGeometryImageFilter has known computational inefficiencies and bugs such as some attributes are not computed with respect to image geometry.

This class is currently in the Review module and is planned to be moved to the Compatibility group under the Deprecated module.

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)

Refer to the ITK Software Guide for
further development details if necessary.

The LabelGeometryImageFilter has known computational inefficiencies
and bugs such as some attributes are not computed with respect to
image geometry.

This class is currently in the Review module and is planned to be
moved to the Compatibility group under the Deprecated module.
@github-actions github-actions bot added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Apr 30, 2024
@blowekamp
Copy link
Member Author

blowekamp commented Apr 30, 2024

The intention is to produce a warning for the current up common release so that with 5.0 6.0 the class can be moved to the deprecated module.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Looks good. You mean to be moved to Deprecated module in 6.0, not 5.0?

@blowekamp
Copy link
Member Author

It does not look like the the ITKReview module has recently receive style changes.

@dzenanz
Copy link
Member

dzenanz commented Apr 30, 2024

Maybe because it is off by default?

@dzenanz
Copy link
Member

dzenanz commented Apr 30, 2024

Perhaps we should turn it on by default (low effort), of finish migration of content out of it and into other corresponding modules (high effort)?

@gdevenyi
Copy link
Contributor

@ntustison @cookpa

@blowekamp
Copy link
Member Author

Perhaps we should turn it on by default (low effort), of finish migration of content out of it and into other corresponding modules (high effort)?

I think turning it on by default may encourage more usage, and limit options available to the remaining classes. I Think the options for the classes in the Review module should be 1) Integrate in ITK proper after changes, 2) make a remote module 3) move to the Deprecate module.

@thewtex thewtex added this to the ITK 6.0 Beta 1 milestone May 1, 2024
@@ -66,6 +66,8 @@ namespace itk
* by Padfield D., Miller J
* https://www.insight-journal.org/browse/publication/301
*
* This class contains computational inefficiencies and bugs such as some attributes are not computed with respect to
* image geometry, consider using these supported alternatives:
Copy link
Member

Choose a reason for hiding this comment

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

👍

@blowekamp
Copy link
Member Author

@thewtex I see you tagged ITK6.0 beta with this PR. So what would be the scheduled to move this class the the ITK Deprecated module? ITK 7.0?

This filter is only in ITKReview, so perhaps less notification is needed?

@thewtex
Copy link
Member

thewtex commented May 2, 2024

This filter is only in ITKReview, so perhaps less notification is needed?

ITKReview is relied on in practice, but it would be helpful to have it marked as deprecated if you would like to move to ITKDeprecated in ITK 6. Merged.

@thewtex thewtex modified the milestones: ITK 6.0 Beta 1, ITK 5.4.0 May 2, 2024
@thewtex thewtex merged commit adb1b5c into InsightSoftwareConsortium:master May 2, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants