ENH: Replace SmallHoleFiller with pct::HoleFillingImageFilter#60
ENH: Replace SmallHoleFiller with pct::HoleFillingImageFilter#60axel-grc wants to merge 3 commits intoRTKConsortium:mainfrom
Conversation
axel-grc
force-pushed
the
pctHoleFillingImageFilter
branch
from
87ff333 to
c42d655
Compare
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
Looks good overall, just minor suggestions.
include/pctHoleFillingImageFilter.h
Outdated
| * is reached or an iteration makes no progress. | ||
| */ | ||
| template <typename TImage> | ||
| class HoleFillingImageFilter : public itk::ImageToImageFilter<TImage, TImage> |
There was a problem hiding this comment.
Can it / is there a benefit to make it an InPlaceImageFilter?
There was a problem hiding this comment.
I think this is not a good idea to use InPlaceImageFilter when working with snapshots
| } | ||
|
|
||
| // Allocate output as a copy of input | ||
| using InitialDuplicatorType = itk::ImageDuplicator<ImageType>; |
There was a problem hiding this comment.
Can be removed with an auto keyword.
|
Thanks for the review @SimonRit, I will propose a new version |
axel-grc
force-pushed
the
pctHoleFillingImageFilter
branch
from
c42d655 to
40440d2
Compare
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
Looks good thanks. Should we add a test? Something like create a small constant image, add holes, and check that the result has constant values? We can merge this one first if you'd like.
|
There is a problem with the git history. If I do the commit presents it as a new file. I would much prefer if you had used to be able to check the changes. Do you mind changing this? I can do it if you'd like. |
I will do it. |
axel-grc
force-pushed
the
pctHoleFillingImageFilter
branch
3 times, most recently
from
6ee5caa to
5f58cf4
Compare
axel-grc
force-pushed
the
pctHoleFillingImageFilter
branch
2 times, most recently
from
9db8191 to
f660561
Compare
SimonRit
force-pushed
the
pctHoleFillingImageFilter
branch
from
f660561 to
fda0792
Compare
2 participants
Close #56