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

BUG: GenerateImageSource sets Size from ReferenceImage #5088

Merged
merged 1 commit into from
Dec 20, 2024
Merged

BUG: GenerateImageSource sets Size from ReferenceImage #5088

merged 1 commit into from
Dec 20, 2024

Conversation

thewtex
Copy link
Member

@thewtex thewtex commented Dec 17, 2024

We want the same behavior as SetOutputParametersFromImage.

Without this, a crash occurs on Update().

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Filtering Issues affecting the Filtering module labels Dec 17, 2024
blowekamp
blowekamp reviewed Dec 17, 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.

Good find. Nice test. I think it could use a tweak though.

Modules/Filtering/ImageSources/include/itkGenerateImageSource.hxx Outdated Show resolved Hide resolved
@dzenanz
Copy link
Member

dzenanz commented Dec 17, 2024

For posterity, this was reported on the forum:
https://discourse.itk.org/t/using-set-setuse-referenceimage-for-gridimagesource-in-itk-python/7353

@thewtex
BUG: GenerateImageSource sets Size from ReferenceImage
c54f1b1 
We want the same behavior as SetOutputParametersFromImage.

Without this, a crash occurs on Update().
@thewtex thewtex force-pushed the generate-reference-size branch from 2758a8b to c54f1b1 Compare December 17, 2024 22:14
@hjmjohnson
Copy link
Member

@thewtex Looks like a fix missed the clang-format fix.

hjmjohnson
hjmjohnson requested changes Dec 18, 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.

clang-format linting failed.

blowekamp
blowekamp approved these changes Dec 18, 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.

This PR is for the release-5.4 branch. The clang-format linter CI is broken on the release branches.

@thewtex
Copy link
Member Author

thewtex commented Dec 20, 2024

Yes, the clang-format linter CI failure is unrelated. Fix being tested in #5102

@thewtex thewtex merged commit ec8c20c into InsightSoftwareConsortium:release-5.4 Dec 20, 2024
16 of 17 checks passed
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 Awaiting requested review from hjmjohnson

Assignees
No one assigned
Labels
area:Filtering Issues affecting the Filtering module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thewtex @dzenanz @hjmjohnson @blowekamp