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.

Sign up for GitHub

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

Jump to bottom

Add and use ProcessObject::MakeRequiredOutputs #4688

Merged
merged 3 commits into from
Jun 18, 2024
Merged

Add and use ProcessObject::MakeRequiredOutputs #4688

merged 3 commits into from
Jun 18, 2024

Conversation

N-Dekker
Copy link
Contributor

@github-actions github-actions bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Segmentation Issues affecting the Segmentation module area:Numerics Issues affecting the Numerics module labels May 21, 2024
@N-Dekker N-Dekker force-pushed the SetRequiredOutputs branch from c0c14a0 to b23ad5b Compare May 21, 2024 17:10
@github-actions github-actions bot added the area:Registration Issues affecting the Registration module label May 21, 2024
@N-Dekker N-Dekker force-pushed the SetRequiredOutputs branch 4 times, most recently from cafc8f5 to 5d3c7ca Compare May 22, 2024 12:18
@N-Dekker
Copy link
Contributor Author

/azp run ITK.Windows

N-Dekker added 3 commits May 23, 2024 12:18
@N-Dekker
ENH: Add protected helper function, ProcessObject::MakeRequiredOutputs
20ce8fb 
Meant to be used in constructors of derived classes that would otherwise
directly call `MakeOutput` for each of their required outputs.
@N-Dekker
ENH: Declare MakeOutput member function of three filters public
f249ac5 
Paved the way for those filters to use `ProcessObject::MakeRequiredOutputs`.

`MakeOutput` member functions are usually declared public, rather than protected.
@N-Dekker
STYLE: Simplify code by calling ProcessObject::MakeRequiredOutputs
ddbed4a 
Reduced amount of duplicate code in constructors that "make" all of their
required outputs.
@N-Dekker N-Dekker force-pushed the SetRequiredOutputs branch from 5d3c7ca to ddbed4a Compare May 23, 2024 10:24
@N-Dekker N-Dekker changed the title Add and use ProcessObject::SetRequiredOutputs Add and use ProcessObject::MakeRequiredOutputs May 23, 2024
@N-Dekker N-Dekker marked this pull request as ready for review May 23, 2024 13:19
@thewtex thewtex requested a review from blowekamp May 31, 2024 14:34
hjmjohnson
hjmjohnson approved these changes Jun 6, 2024
View reviewed changes
Copy link
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

LGTM.

@hjmjohnson
Copy link
Member

@blowekamp Brad would you mind double checking that this looks OK?

blowekamp
blowekamp requested changes Jun 12, 2024
View reviewed changes
Copy link
Member

@blowekamp blowekamp left a comment

Choose a reason for hiding this comment

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

There are some cases there the correct derive MakeOutput is no longer called.

Modules/Filtering/DistanceMap/include/itkDanielssonDistanceMapImageFilter.hxx Show resolved Hide resolved
@hjmjohnson hjmjohnson merged commit abb5928 into InsightSoftwareConsortium:master Jun 18, 2024
14 checks passed
This was referenced Jun 21, 2024
Closed
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blowekamp blowekamp blowekamp approved these changes

@hjmjohnson hjmjohnson hjmjohnson approved these changes

Assignees
No one assigned
Labels
area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@N-Dekker @hjmjohnson @blowekamp