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

Support Python dispatching on PrimaryInputName #4858

Closed
thewtex opened this issue Sep 23, 2024 · 7 comments
Closed

Support Python dispatching on PrimaryInputName #4858

thewtex opened this issue Sep 23, 2024 · 7 comments
Assignees
Labels
type:Enhancement Improvement of existing methods or implementation
Milestone

Comments

@thewtex
Copy link
Member

thewtex commented Sep 23, 2024

We should dispatch on PrimaryInputName when it is passed as a keyword argument instead of a positional parameter.

xref:

CC: @N-Dekker @mstaring

@thewtex thewtex added the type:Enhancement Improvement of existing methods or implementation label Sep 23, 2024
@thewtex thewtex added this to the ITK 5.4.1 milestone Sep 23, 2024
@thewtex thewtex self-assigned this Sep 23, 2024
@N-Dekker
Copy link
Contributor

@thewtex I'm glad you are looking at this issue. Is the following supposed to work well?

    output_image = itk.median_image_filter(input=my_float_image)

(It now says TypeError: Expecting argument of type itkImageSS2 or itkImageSourceISS2). Or should the keyword argument start with a capital, Input=my_float_image? I'm asking, because "Input" is in the list of so-called primary_input_methods at:

primary_input_methods = ("Input", "InputImage", "Input1")

And primary_input_methods is used for dispatching:

elif set(primary_input_methods).intersection(kwargs.keys()):
for method in primary_input_methods:
if method in kwargs:
input_type = output(kwargs[method]).__class__
keys = ttype_for_input_type(keys, input_type)
break

Regarding ITKElastix issue InsightSoftwareConsortium/ITKElastix#212, obviously, "FixedImage" and "MovingImage" are not in primary_input_methods (and neither are "fixed_image" and "moving_image"). So I guess primary_input_methods should be extended, right?

@blowekamp
Copy link
Member

The Process object maintains a lists of index inputs and named inputs. And a named input can be marked as primary. These named inputs are already coded input many filters e.g.
PDEDeformableRegistrationFilter

The information in the Process object should likely be used.

@N-Dekker
Copy link
Contributor

Thanks @blowekamp I see, PDEDeformableRegistrationFilter has inputs named "FixedImage" and "MovingImage":

// #1 "FixedImage" required
Self::AddRequiredInputName("FixedImage", 1);
// #2 "MovingImage" required
Self::AddRequiredInputName("MovingImage", 2);

So then, should something like the following work, for itk::Image<float> objects float_image1 and float_image2?

itk.pde_deformable_registration_filter(FixedImage=float_image1, MovingImage=float_image2)

(It now says TypeError: Expecting argument of type itkImageSS2 or itkImageSourceISS2)

@dzenanz
Copy link
Member

dzenanz commented Oct 17, 2024

In procedural interface, the parameters get converted to snake_case, so try this: itk.pde_deformable_registration_filter(fixed_image=float_image1, moving_image=float_image2).

@blowekamp
Copy link
Member

I would expect ProcessObject::GetPrimaryInputName to be useful.

I am not too sure of these details in ITK Python. Specifically if the Process object's name inputs/outputs are used or if it's just mapping keyword arguments to methods.

Also I think that PDE deformable registration class may be a base class for the deacons methods, and I have a harder guess of which is the primary input, and how the template parameters get mapped from it.

@thewtex
Copy link
Member Author

thewtex commented Oct 17, 2024

Is the following supposed to work well?

    output_image = itk.median_image_filter(input=my_float_image)

The keyword here to be supported would be the snake_case version of itk::MedianImageFilter::GetPrimaryInputName();

thewtex added a commit to thewtex/ITK that referenced this issue Oct 30, 2024
Dispatch Python filter types based on the type of the first
ProcessObject RequiredInputName, which is exposed in Python via
GetRequiredInputName and which can be the ProcessObject
PrimaryInputName.

This helps to address uses cases such as
`itk.elastix_registration_method`, where you still want to infer the
filter type based on the input fixed image type, but it may be passed in
as a keyword argument, such as
`itk.elastix_registration_method(moving_image=moving_image, fixed_image=fixed_image)`.

Re: InsightSoftwareConsortium#4858 InsightSoftwareConsortium/ITKElastix#212
thewtex added a commit to thewtex/ITK that referenced this issue Nov 4, 2024
Dispatch Python filter types based on the type of the first
ProcessObject RequiredInputName, which is exposed in Python via
GetRequiredInputName and which can be the ProcessObject
PrimaryInputName.

This helps to address uses cases such as
`itk.elastix_registration_method`, where you still want to infer the
filter type based on the input fixed image type, but it may be passed in
as a keyword argument, such as
`itk.elastix_registration_method(moving_image=moving_image, fixed_image=fixed_image)`.

Re: InsightSoftwareConsortium#4858 InsightSoftwareConsortium/ITKElastix#212
@thewtex
Copy link
Member Author

thewtex commented Nov 5, 2024

Will be closed via #4921

@thewtex thewtex closed this as completed Nov 5, 2024
@thewtex thewtex reopened this Nov 5, 2024
thewtex added a commit to thewtex/ITK that referenced this issue Nov 7, 2024
Dispatch Python filter types based on the type of the first
ProcessObject RequiredInputName, which is exposed in Python via
GetRequiredInputName and which can be the ProcessObject
PrimaryInputName.

This helps to address uses cases such as
`itk.elastix_registration_method`, where you still want to infer the
filter type based on the input fixed image type, but it may be passed in
as a keyword argument, such as
`itk.elastix_registration_method(moving_image=moving_image, fixed_image=fixed_image)`.

Re: InsightSoftwareConsortium#4858 InsightSoftwareConsortium/ITKElastix#212
hjmjohnson pushed a commit that referenced this issue Nov 8, 2024
Dispatch Python filter types based on the type of the first
ProcessObject RequiredInputName, which is exposed in Python via
GetRequiredInputName and which can be the ProcessObject
PrimaryInputName.

This helps to address uses cases such as
`itk.elastix_registration_method`, where you still want to infer the
filter type based on the input fixed image type, but it may be passed in
as a keyword argument, such as
`itk.elastix_registration_method(moving_image=moving_image, fixed_image=fixed_image)`.

Re: #4858 InsightSoftwareConsortium/ITKElastix#212
@thewtex thewtex closed this as completed Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Enhancement Improvement of existing methods or implementation
Projects
None yet
Development

No branches or pull requests

4 participants