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

Object returned by reference are not `const` in Python #335

Open
fbudin69500 opened this Issue Dec 20, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@fbudin69500
Copy link
Contributor

fbudin69500 commented Dec 20, 2018

Description

Some ITK objects that belong to another object are returned by reference and are not const in Python. This means that the user can modify them, and then be surprised by the values contained in the object not corresponding to the values they expect.

Steps to Reproduce

As an example, here is a short snippet of Python code to reproduce the issue:

    size_im=image.GetLargestPossibleRegion().GetSize()
    new_size=itk.Size.x2()
    print type(size_im)
    for i in range(0,len(size_im)):
        new_size.SetElement(i, int(size_im[i]/2)+1)
        size_im.SetElement(i, int(size_im[i]/2)+1)
    print new_size
    print size_im

    gaussianImage = itk.DiscreteGaussianImageFilter(image, UseImageSpacing=True, Variance=variance)
    print "after gaussian:"+str(new_size)
    resampledImage=itk.ResampleImageFilter(gaussianImage, Size=new_size, OutputSpacing=spacing_new_image)
    print itk.size(resampledImage)
    print size_im

Expected behavior

The behavior I would have expected is that the size of the image returned by print size_im at the end would match the size returned by print itk.size(resampledImage).

Actual behavior

print size_im prints the original image size, not the one after manual modification.

Reproducibility

All the time

Versions

4.13.1 for sure, and most likely any version 5.0 based on the fact that the C++ code returns a reference, not a value (in C++ the reference is const).

Additional Information

Here are links to (some?) culprit functions:
https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/include/itkImageRegion.h#L161
https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/include/itkImageBase.h#L251

@fbudin69500

This comment has been minimized.

Copy link
Contributor

fbudin69500 commented Dec 20, 2018

@thewtex This problem is most likely also a problem in C++, but I have never encountered it before. In C++, the returned reference is const

@dzenanz

This comment has been minimized.

Copy link
Member

dzenanz commented Dec 20, 2018

In C++ compiler doesn't let you modify a const reference without const_cast. And if you do that you are not surprised that you get unexpected behavior. Python wrapping maybe throws away the constness part of the reference?

Solution might be to return a copy instead of a reference whenever const reference is encountered.

@blowekamp

This comment has been minimized.

Copy link
Member

blowekamp commented Dec 20, 2018

I also believe a similar thing happens with constant smart pointers. They are not constant either.

@fbudin69500 fbudin69500 changed the title Object returned by reference and not `const` Object returned by reference are not `const` in Python Dec 20, 2018

@fbudin69500

This comment has been minimized.

Copy link
Contributor

fbudin69500 commented Dec 20, 2018

In Python, the Smart Pointers are replaced by pointers (at least often), so that could also explain why the Smart pointers are not const. Maybe the const qualifier is removed at that point too.

@thewtex

This comment has been minimized.

Copy link
Member

thewtex commented Dec 20, 2018

Python passes objects by reference and does not have const, which are two things that are commonly encountered gotchas for Python in general.

In this case, I think we can help the situation by making itk.size, etc., return a copy.

@blowekamp

This comment has been minimized.

Copy link
Member

blowekamp commented Dec 20, 2018

I had a similar issue with accidentally modifying a pipeline images Largest possible region which trying to stream read. This was very hard to track down.

I wonder if for types like ImageRegion you can tell SWIG to do a copy instead of using a reference.

@fbudin69500

This comment has been minimized.

Copy link
Contributor

fbudin69500 commented Dec 20, 2018

yes, returning a copy might be the solution since Python doesn't have a const qualifier. SWIG already does that automatically for primitive datatypes.

" However, functions that return const references to primitive datatypes (int, short, etc.) normally return the result as a value rather than a pointer. For example, a function like this, "

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment