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

Add method argument to rotate and affine_transform #5916

Merged
merged 1 commit into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/5916.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
`sunpy.image.transform.affine_transform` and :meth:`sunpy.map.GenericMap.rotate`
have both had their ``use_scipy`` arguments deprecated. Instead use the new
``method`` argument, which can currently be set to either ``'skimage'``
(the default) or ``'scipy'``. In the future we hope to use this new argument
to support generic user supplied transformation functions.
23 changes: 19 additions & 4 deletions sunpy/image/tests/test_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from skimage import transform as tf

from sunpy.image.transform import affine_transform
from sunpy.util import SunpyUserWarning
from sunpy.util import SunpyDeprecationWarning, SunpyUserWarning

# Tolerance for tests
RTOL = 1.0e-10
Expand Down Expand Up @@ -80,14 +80,14 @@ def test_scipy_rotation(original, angle, k):
s = np.round(np.sin(angle))
rmatrix = np.array([[c, -s], [s, c]])
expected = np.rot90(original, k=k)
rot = affine_transform(original, rmatrix=rmatrix, use_scipy=True)
rot = affine_transform(original, rmatrix=rmatrix, method='scipy')
assert compare_results(expected, rot, allclose=False)

# TODO: Check incremental 360 degree rotation against original image

# Check derotated image against original
derot_matrix = np.array([[c, s], [-s, c]])
derot = affine_transform(rot, rmatrix=derot_matrix, use_scipy=True)
derot = affine_transform(rot, rmatrix=derot_matrix, method='scipy')
assert compare_results(original, derot, allclose=False)


Expand Down Expand Up @@ -225,7 +225,7 @@ def test_nan_scipy(identity):
# Test replacement of NaN values for scipy rotation
in_arr = np.array([[np.nan]])
with pytest.warns(SunpyUserWarning, match='Setting NaNs to 0 for SciPy rotation.'):
out_arr = affine_transform(in_arr, rmatrix=identity, use_scipy=True)
out_arr = affine_transform(in_arr, rmatrix=identity, method='scipy')
assert not np.all(np.isnan(out_arr))


Expand All @@ -245,6 +245,21 @@ def test_float32(identity):
assert np.issubdtype(out_arr.dtype, np.float32)


def test_deprecated_args(identity):
in_arr = np.array([[100]])
with pytest.warns(SunpyDeprecationWarning, match="The 'use_scipy' argument is deprecated"):
out_arr = affine_transform(in_arr, rmatrix=identity, use_scipy=True)

with pytest.warns(SunpyDeprecationWarning, match="The 'use_scipy' argument is deprecated"):
out_arr = affine_transform(in_arr, rmatrix=identity, use_scipy=False)

with pytest.raises(ValueError, match="Method blah not in supported methods"):
out_arr = affine_transform(in_arr, rmatrix=identity, method='blah')

with pytest.warns(SunpyUserWarning, match="Using scipy instead of skimage for rotation"):
out_arr = affine_transform(in_arr, rmatrix=identity, use_scipy=True, method='skimage')


def test_reproducible_matrix_multiplication():
# Test whether matrix multiplication involving a large matrix always gives the same answer
# This indirectly tests whichever BLAS/LAPACK libraries that NumPy is linking to (if any)
Expand Down
49 changes: 36 additions & 13 deletions sunpy/image/transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
import numpy as np
import scipy.ndimage

from sunpy.util.exceptions import warn_user
from sunpy.util.exceptions import warn_deprecated, warn_user

__all__ = ['affine_transform']


def affine_transform(image, rmatrix, order=3, scale=1.0, image_center=None,
recenter=False, missing=0.0, use_scipy=False):
recenter=False, missing=0.0, use_scipy=None, *, method='skimage'):
"""
Rotates, shifts and scales an image.

Expand Down Expand Up @@ -40,10 +40,11 @@ def affine_transform(image, rmatrix, order=3, scale=1.0, image_center=None,
Defaults to `True` i.e., recenter to the center of the array.
missing : `float`, optional
The value to replace any missing data after the transformation.
use_scipy : `bool`, optional
Force use of `scipy.ndimage.affine_transform`.
Will set all "NaNs" in image to zero before doing the transform.
Defaults to `False`, unless scikit-image can't be imported.
method : {'skimage', 'scipy'}
Transform function to use. Currently
:func:`scipy.ndimage.affine_transform` and
:func:`skimage.transform.warp` are supported.
Defaults to 'skimage', unless scikit-image can't be imported.

Returns
-------
Expand Down Expand Up @@ -93,20 +94,18 @@ def affine_transform(image, rmatrix, order=3, scale=1.0, image_center=None,

displacement = np.dot(rmatrix, rot_center)
shift = image_center - displacement
if not use_scipy:
try:
import skimage.transform
except ImportError:
warn_user("scikit-image could not be imported. Image rotation will use scipy.")
use_scipy = True
if use_scipy:

method = _get_transform_method(method, use_scipy)
if method == 'scipy':
if np.any(np.isnan(image)):
warn_user("Setting NaNs to 0 for SciPy rotation.")
# Transform the image using the scipy affine transform
rotated_image = scipy.ndimage.affine_transform(
np.nan_to_num(image).T, rmatrix, offset=shift, order=order,
mode='constant', cval=missing).T
else:
import skimage.transform

# Make the rotation matrix 3x3 to include translation of the image
skmatrix = np.zeros((3, 3))
skmatrix[:2, :2] = rmatrix
Expand Down Expand Up @@ -148,3 +147,27 @@ def affine_transform(image, rmatrix, order=3, scale=1.0, image_center=None,
rotated_image += im_min

return rotated_image


def _get_transform_method(method, use_scipy):
# This is re-used in affine_transform and GenericMap.rotate
supported_methods = {'scipy', 'skimage'}
if method not in supported_methods:
raise ValueError(f'Method {method} not in supported methods: {supported_methods}')

if use_scipy is not None:
nabobalis marked this conversation as resolved.
Show resolved Hide resolved
warn_deprecated("The 'use_scipy' argument is deprecated. "
"Specify the rotation method to the 'method' "
"keyword argument instead.")
if use_scipy is True and method != 'scipy':
warn_user(f"Using scipy instead of {method} for rotation.")
method = 'scipy'

if method == 'skimage':
try:
import skimage # NoQA
except ImportError:
warn_user("scikit-image could not be imported. Image rotation will use scipy.")
method = 'scipy'

return method
17 changes: 9 additions & 8 deletions sunpy/map/mapbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ def resample(self, dimensions: u.pixel, method='linear'):

@u.quantity_input
def rotate(self, angle: u.deg = None, rmatrix=None, order=4, scale=1.0,
recenter=False, missing=0.0, use_scipy=False):
recenter=False, missing=0.0, use_scipy=None, *, method='skimage'):
"""
Returns a new rotated and rescaled map.

Expand Down Expand Up @@ -1557,11 +1557,10 @@ def rotate(self, angle: u.deg = None, rmatrix=None, order=4, scale=1.0,
missing : float
The numerical value to fill any missing points after rotation.
Default: 0.0
use_scipy : bool
If True, forces the rotation to use
:func:`scipy.ndimage.affine_transform`, otherwise it
uses the :func:`skimage.transform.warp`.
Default: False, unless scikit-image can't be imported
method : {'skimage', 'scipy'}
Rotation function to use. Currently
:func:`scipy.ndimage.affine_transform` and
:func:`skimage.transform.warp` are supported.

Returns
-------
Expand All @@ -1584,7 +1583,7 @@ def rotate(self, angle: u.deg = None, rmatrix=None, order=4, scale=1.0,
to rotation, and differences from IDL's rot().
"""
# Put the import here to reduce sunpy.map import time
from sunpy.image.transform import affine_transform
from sunpy.image.transform import _get_transform_method, affine_transform

if angle is not None and rmatrix is not None:
raise ValueError("You cannot specify both an angle and a rotation matrix.")
Expand All @@ -1594,6 +1593,8 @@ def rotate(self, angle: u.deg = None, rmatrix=None, order=4, scale=1.0,
if order not in range(6):
raise ValueError("Order must be between 0 and 5.")

method = _get_transform_method(method, use_scipy)

# The FITS-WCS transform is by definition defined around the
# reference coordinate in the header.
lon, lat = self._get_lon_lat(self.reference_coordinate.frame)
Expand Down Expand Up @@ -1644,7 +1645,7 @@ def rotate(self, angle: u.deg = None, rmatrix=None, order=4, scale=1.0,
order=order, scale=scale,
image_center=np.flipud(pixel_center),
recenter=recenter, missing=missing,
use_scipy=use_scipy).T
method=method).T

if recenter:
new_reference_pixel = pixel_array_center
Expand Down