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

MRG: add mode and cval arguments for resampling #359

Merged
merged 11 commits into from
Sep 25, 2015

Conversation

matthew-brett
Copy link
Member

We sometimes need to resample images with specific ndimage resampling modes,
such as changing the constant values outside the boundary or the boundary
interpolation modes.

Add and test these arguments to the nipy resampling functions.

jbpoline and others added 4 commits September 5, 2015 16:54
Whitespace and docstring cleanup for resample, interplation edits.
Fix bug where arguments were not being passed to interpolation routine
from `resample_img2img`.

Extend tests and remove commented tests.

Add tests for interpolator
@matthew-brett matthew-brett mentioned this pull request Sep 6, 2015
@matthew-brett
Copy link
Member Author

This is #309 with some style and docstring edits, a fix, and some extra tests.

@matthew-brett
Copy link
Member Author

@jbpoline - do you have time for a review?

Transform object accidentally enforced `__init__` signature, but using
``self.__class__``.  Use ``self.apply`` and return object of class
``Transform`` so we can have subclasses with different ``__init__``
signatures.
Not-affine transforms were not being rounded or thresholded correctly
(as was the case for the Cython routine doing resampling for affine
case).

Not-affine, not default order case was not being reshaped correctly.
We were using a dictionary to pass extra kwargs to ndimage. Instead,
define the relevant arguments explicitly.
@matthew-brett
Copy link
Member Author

@alexis-roche - when I was extending the tests, I noticed that the resample function in algorithms/registration/resample.py was broken for not-Affine transforms. I added some fixes and more tests. Would you mind taking a look?

@alexis-roche
Copy link
Member

I am proposing a patch to make the cython cubic spline resampling routine signature consistent with the simple sampling routine (_cspline_sample) used in the non-affine case. It did not make sense that the casting were handled at different levels depending on whether the transform was affine or not. The patch remedies this inconsistency:

https://github.com/alexis-roche/nipy/tree/matthew-resample

@matthew-brett
Copy link
Member Author

Do you also want to threshold the peak values so they cast correctly? There's stuff in nibabel for that :

from nibabel.casting import shared_range

mn, mx = shared_range(np.double, out_dtype)
res = np.clip(out, mn, mx).astype(out_dtype)

That ensures the top values don't wrap around into negatives etc.

@matthew-brett
Copy link
Member Author

Would you mind making your patch a PR against my branch?

@alexis-roche
Copy link
Member

Not sure users would expect clipping in the context of resampling an image, where the dynamic range is not expected to change significantly. I rather assume otherwise.

@matthew-brett
Copy link
Member Author

Probably the user wouldn't expect this though:

In [4]: a = np.array([0., 257.])

In [5]: a.astype(np.uint8)
Out[5]: array([0, 1], dtype=uint8)

@alexis-roche
Copy link
Member

Oh sorry - you mean the problem of wrapping around negative values. Yes, it would make sense to avoid that. But it should not affect intensities within the 'castable' range.

@matthew-brett
Copy link
Member Author

Right - anything outside the castable range with do terrible things with a naive cast.

matthew-brett added a commit that referenced this pull request Sep 25, 2015
MRG: add mode and cval arguments for resampling

We sometimes need to resample images with specific ndimage resampling modes,
such as changing the constant values outside the boundary or the boundary
interpolation modes.

Add and test these arguments to the nipy resampling functions.

Fix some bugs in casting when using non-affines for resampling.
@matthew-brett matthew-brett merged commit a0b472a into nipy:master Sep 25, 2015
@matthew-brett
Copy link
Member Author

Thanks - sorry to be slow to get back to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants