ENH: Set Seeds as conntainer objects#4793
Conversation
|
@jhlegarreta I was surprised to see These were the quick changes I made to have the seeds list behave like objects with setter/getters. Further investigation found other region growing filters with similar interfaces:
These classes seem to use std::vector to hold the seed lists. Perhaps performance was a reason for this approach? Modern C++ could do something like What can be done to improve the interface of these filters and make them more consistent? |
jhlegarreta
left a comment
There was a problem hiding this comment.
Really sorry @blowekamp, looks like I was confused by the docstring in the implementation file, which documented the method as being deprecated:
d58bf56#diff-9daea1a5d3498baf38e16ef9fd06da89d2f898c4b599a7eb287a76fc3a720accL105
d58bf56#diff-9daea1a5d3498baf38e16ef9fd06da89d2f898c4b599a7eb287a76fc3a720accL137
But after investigation, I now see that the documentation was not updated and the passage left inadvertently in
c005b54
So please, go ahead and update the documentation.
If possible, can the methods be tested, please?
d5a4ea0 to
6ecbb73
Compare
|
Rebased onto current Details
|
6ecbb73 to
bc2af92
Compare
|
@blowekamp I've rebased your initial work and addressed the comment to add a test. I left this as a draft and kept you as the author. |
hjmjohnson
left a comment
There was a problem hiding this comment.
I'm good with this as is, or with more aggressive deprecation warnings.
533c4fe to
d6596f4
Compare
Add SeedsContainerType alias and itkSetMacro/itkGetConstReferenceMacro for Seeds to ConfidenceConnectedImageFilter, ConnectedThresholdImageFilter, NeighborhoodConnectedImageFilter, and VectorConfidenceConnectedImageFilter. Removes manual virtual GetSeeds() implementations in favour of the macro-generated accessors. Consistent with IsolatedConnectedImageFilter.
d6596f4 to
de01678
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
@blowekamp Greptile suggestions look like appropriate minor improvements. |
…ters Add round-trip smoke tests for SetSeeds/GetSeeds to ConfidenceConnectedImageFilter, ConnectedThresholdImageFilter, NeighborhoodConnectedImageFilter, and VectorConfidenceConnectedImageFilter tests. Verifies ClearSeeds empties the container and SetSeeds/GetSeeds round-trip correctly.
|
/azp run ITK.Linux |
|
/azp run ITK.Linux.Python |
|
/azp run ITK.macOS.Python |
Best practices are to set member variables as object, and not repeat accessor methods in the interface of the owning object e.g. AddSeed, ClearSeed.
PR Checklist
Refer to the ITK Software Guide for
further development details if necessary.