Skip to content

Commit

Permalink
Raise our own error when trying to rotate a map of integer data while…
Browse files Browse the repository at this point in the history
… using a `missing` value of NaN (#7344)

* Update mapbase.py

try...except used to provide a useful error message to users.

* Update sunpy/map/mapbase.py

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

* Update sunpy/map/mapbase.py

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

* Update sunpy/map/mapbase.py

* Update sunpy/map/mapbase.py

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

* Update sunpy/map/mapbase.py

* added unit test for mapbase file change test_mapbase.py

changed unit test according to the try..except added to the padding

* unit test added to make sure we hit the error in test_mapbase.py

* Update test_mapbase.py

added unit test with raising a warning rather than an error in padding

* skipif removed in incompatible_missing_dtype_error()

* Update sunpy/map/mapbase.py

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

* added a common unit test for both error and warning in numpy padding.

* new_data variable introduced for except method aswell

* if condition for numpy <1.20.0 removed

* warning condition removed in unit testing and value error unit testing done

* raising value error after except in mapbase.py

* Update mapbase.py

* Create 7344.feature.rst

* Update mapbase.py

* Update test_mapbase.py

* Update test_mapbase.py

* Update mapbase.py

* pre-commits fixed

* faliures resolved

* Update changelog/7344.feature.rst

Co-authored-by: Albert Y. Shih <ayshih@gmail.com>

* Update sunpy/map/tests/test_mapbase.py

Co-authored-by: Albert Y. Shih <ayshih@gmail.com>

* Update sunpy/map/mapbase.py

Co-authored-by: Albert Y. Shih <ayshih@gmail.com>

* Update sunpy/map/mapbase.py

Co-authored-by: Albert Y. Shih <ayshih@gmail.com>

* suggestions commited

* pre-commit resolved

* logic fixed and test case updated for mapbase.py

---------

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
Co-authored-by: Albert Y. Shih <ayshih@gmail.com>
  • Loading branch information
3 people committed Dec 25, 2023
1 parent e000af8 commit e2a0942
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 29 deletions.
1 change: 1 addition & 0 deletions changelog/7344.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When calling :meth:`sunpy.map.GenericMap.rotate` on an integer data array, with ``missing`` set to NaN (the default value), the method will now itself raise an informative error message instead deferring to NumPy to raise the error.
20 changes: 10 additions & 10 deletions sunpy/map/mapbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import numpy as np
from matplotlib.backend_bases import FigureCanvasBase
from matplotlib.figure import Figure
from packaging import version

try:
from dask.array import Array as DaskArray
Expand Down Expand Up @@ -1701,16 +1700,17 @@ def rotate(self, angle: u.deg = None, rmatrix=None, order=3, scale=1.0,
unpad_x = -np.min((diff[0], 0))
unpad_y = -np.min((diff[1], 0))

# Numpy 1.20+ prevents np.pad from padding with NaNs in integer arrays
# Before it would be cast to 0, but now it raises an error.
if version.parse(np.__version__) < version.parse("1.20.0") and issubclass(self.data.dtype.type, numbers.Integral) and (missing % 1 != 0):
warn_user("The specified `missing` value is not an integer, but the data "
"array is of integer type, so the output may be strange.")
# Pad the image array
# Raise an informative error message if trying to pad an integer array with NaNs
if (pad_x > 0 or pad_y > 0) and issubclass(self.data.dtype.type, numbers.Integral) and (missing % 1 != 0):
raise ValueError("The underlying data is integers, but the fill value for missing "
"pixels cannot be cast to an integer, which is the case for the "
"default fill value of NaN. Set the `missing` keyword to an "
"appropriate integer value for the data set.")

new_data = np.pad(self.data,
((pad_y, pad_y), (pad_x, pad_x)),
mode='constant',
constant_values=(missing, missing))
((pad_y, pad_y), (pad_x, pad_x)),
mode='constant',
constant_values=(missing, missing))

# All of the following pixel calculations use a pixel origin of 0
pixel_array_center = (np.flipud(new_data.shape) - 1) / 2.0
Expand Down
22 changes: 3 additions & 19 deletions sunpy/map/tests/test_mapbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import pytest
from hypothesis import HealthCheck, given, settings
from matplotlib.figure import Figure
from packaging import version

import astropy.units as u
import astropy.wcs
Expand Down Expand Up @@ -1028,30 +1027,15 @@ def test_rotate(aia171_test_map):
assert aia171_test_map_crop_rot.data.shape[0] < aia171_test_map_crop_rot.data.shape[1]


@pytest.mark.xfail(version.parse(np.__version__) >= version.parse("1.20.0"),
reason="Numpy >= 1.20.0 doesn't allow NaN to int conversion")
def test_rotate_with_incompatible_missing_dtype_warning():
data = np.arange(0, 100).reshape(10, 10)
coord = SkyCoord(0 * u.arcsec, 0 * u.arcsec, obstime='2013-10-28',
observer='earth', frame=sunpy.coordinates.Helioprojective)
header = sunpy.map.make_fitswcs_header(data, coord)
test_map = sunpy.map.Map(data, header)
with pytest.warns(SunpyUserWarning,
match="The specified `missing` value is not an integer, but the data "
"array is of integer type, so the output may be strange."):
test_map.rotate(order=3, missing=np.nan)


@pytest.mark.skipif(version.parse(np.__version__) <= version.parse("1.20.0"),
reason="Numpy >= 1.20.0 doesn't allow NaN to int conversion")
def test_rotate_with_incompatible_missing_dtype_error():
data = np.arange(0, 100).reshape(10, 10)
coord = SkyCoord(0 * u.arcsec, 0 * u.arcsec, obstime='2013-10-28',
observer='earth', frame=sunpy.coordinates.Helioprojective)
header = sunpy.map.make_fitswcs_header(data, coord)
test_map = sunpy.map.Map(data, header)
with pytest.raises(ValueError, match="cannot convert float NaN to integer"):
test_map.rotate(order=3, missing=np.nan)
with pytest.raises(ValueError, match="The underlying data is integers, but the fill value for "
"missing pixels cannot be cast to an integer"):
test_map.rotate(angle=45 * u.deg, missing=np.nan, order=3)


def test_rotate_crpix_zero_degrees(generic_map):
Expand Down

0 comments on commit e2a0942

Please sign in to comment.