From 420bc806360c5bcbb79a551cc67d6c901d7507fa Mon Sep 17 00:00:00 2001 From: Paige Martin Date: Thu, 26 Aug 2021 18:20:45 +1000 Subject: [PATCH 1/6] Add value error and test for nondim coords --- xrft/tests/test_xrft.py | 22 ++++++++++++++++++++++ xrft/xrft.py | 7 +++++++ 2 files changed, 29 insertions(+) diff --git a/xrft/tests/test_xrft.py b/xrft/tests/test_xrft.py index 482e3432..9441f18a 100644 --- a/xrft/tests/test_xrft.py +++ b/xrft/tests/test_xrft.py @@ -351,6 +351,8 @@ def test_window_single_dim(): ps.load() + + class TestSpectrum(object): @pytest.mark.parametrize("dim", ["t", "time"]) @pytest.mark.parametrize("window_correction", [True, False]) @@ -1306,3 +1308,23 @@ def test_reversed_coordinates(): xrt.assert_allclose( xrft.dft(s, dim="x", true_phase=True), xrft.dft(s2, dim="x", true_phase=True) ) + +def test_nondim_coords(): + """Error should be raised if there are non-dimensional coordinates attached to the dimension(s) over which the FFT is being taken""" + N = 16 + da = xr.DataArray( + np.random.rand(2, N, N), + dims=["time", "x", "y"], + coords={ + "time": np.array(["2019-04-18", "2019-04-19"], dtype="datetime64"), + "x": range(N), + "y": range(N), + 'x_nondim':('x',np.arange(N)) + }, + ) + + with pytest.raises(ValueError): + xrft.power_spectrum(da) + + xrft.power_spectrum(da,dim=['time','y']) + diff --git a/xrft/xrft.py b/xrft/xrft.py index ea4a9e86..c06ab2f3 100644 --- a/xrft/xrft.py +++ b/xrft/xrft.py @@ -383,6 +383,13 @@ def fft( N = [da.shape[n] for n in axis_num] + # raise error if there are multiple coordinates attached to the dimension(s) over which the FFT is taken + for d in dim: + if not da[d].equals(da[d].reset_coords(drop=True)): + raise ValueError( + "This function currently does not handle more than one coordinate attached to the dimension(s) over which the FFT is taken." + ) + # verify even spacing of input coordinates delta_x = [] lag_x = [] From 3a08f471f0d67505481ad9429d400326f25e2afe Mon Sep 17 00:00:00 2001 From: Paige Martin Date: Thu, 26 Aug 2021 18:33:04 +1000 Subject: [PATCH 2/6] Fix minor coding inconsistencies --- xrft/tests/test_xrft.py | 4 ++-- xrft/xrft.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/xrft/tests/test_xrft.py b/xrft/tests/test_xrft.py index 9441f18a..145eba1d 100644 --- a/xrft/tests/test_xrft.py +++ b/xrft/tests/test_xrft.py @@ -1319,12 +1319,12 @@ def test_nondim_coords(): "time": np.array(["2019-04-18", "2019-04-19"], dtype="datetime64"), "x": range(N), "y": range(N), - 'x_nondim':('x',np.arange(N)) + "x_nondim":("x",np.arange(N)) }, ) with pytest.raises(ValueError): xrft.power_spectrum(da) - xrft.power_spectrum(da,dim=['time','y']) + xrft.power_spectrum(da,dim=["time","y"]) diff --git a/xrft/xrft.py b/xrft/xrft.py index c06ab2f3..ab3ae3ea 100644 --- a/xrft/xrft.py +++ b/xrft/xrft.py @@ -388,7 +388,7 @@ def fft( if not da[d].equals(da[d].reset_coords(drop=True)): raise ValueError( "This function currently does not handle more than one coordinate attached to the dimension(s) over which the FFT is taken." - ) + ) # verify even spacing of input coordinates delta_x = [] From 22bef514b4fd2840484ebe9cb33cca76c5ad47b9 Mon Sep 17 00:00:00 2001 From: paigem Date: Fri, 27 Aug 2021 10:26:47 +1000 Subject: [PATCH 3/6] Apply suggestions from code review Clearer, more helpful error message Co-authored-by: Ryan Abernathey --- xrft/xrft.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xrft/xrft.py b/xrft/xrft.py index ab3ae3ea..69c8f63f 100644 --- a/xrft/xrft.py +++ b/xrft/xrft.py @@ -385,7 +385,8 @@ def fft( # raise error if there are multiple coordinates attached to the dimension(s) over which the FFT is taken for d in dim: - if not da[d].equals(da[d].reset_coords(drop=True)): + bad_coords = [cname for cname in da.coords if cname != dim and dim in da[cname].dims] + if bad_coords: raise ValueError( "This function currently does not handle more than one coordinate attached to the dimension(s) over which the FFT is taken." ) From 4bffa9ce76fe1b7bced82b3f539ccfec9e8b6bd1 Mon Sep 17 00:00:00 2001 From: paigem Date: Fri, 27 Aug 2021 10:43:25 +1000 Subject: [PATCH 4/6] Update wording of error message --- xrft/xrft.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xrft/xrft.py b/xrft/xrft.py index 69c8f63f..e0144903 100644 --- a/xrft/xrft.py +++ b/xrft/xrft.py @@ -388,7 +388,8 @@ def fft( bad_coords = [cname for cname in da.coords if cname != dim and dim in da[cname].dims] if bad_coords: raise ValueError( - "This function currently does not handle more than one coordinate attached to the dimension(s) over which the FFT is taken." + f"The input array contains coordinate variable(s) ({bad_coords}) whose dims include the transform dimension(s) `{dim}`. " + f"Please drop these coordinates (`.drop({bad_coords}`) before invoking xrft." ) # verify even spacing of input coordinates From 9f79f077ba98fa9e33e286e359fab4719a578f96 Mon Sep 17 00:00:00 2001 From: Paige Martin Date: Fri, 27 Aug 2021 11:07:53 +1000 Subject: [PATCH 5/6] run code-style checks --- xrft/tests/test_xrft.py | 10 ++++------ xrft/xrft.py | 4 +++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/xrft/tests/test_xrft.py b/xrft/tests/test_xrft.py index 145eba1d..dc661879 100644 --- a/xrft/tests/test_xrft.py +++ b/xrft/tests/test_xrft.py @@ -351,8 +351,6 @@ def test_window_single_dim(): ps.load() - - class TestSpectrum(object): @pytest.mark.parametrize("dim", ["t", "time"]) @pytest.mark.parametrize("window_correction", [True, False]) @@ -1309,6 +1307,7 @@ def test_reversed_coordinates(): xrft.dft(s, dim="x", true_phase=True), xrft.dft(s2, dim="x", true_phase=True) ) + def test_nondim_coords(): """Error should be raised if there are non-dimensional coordinates attached to the dimension(s) over which the FFT is being taken""" N = 16 @@ -1319,12 +1318,11 @@ def test_nondim_coords(): "time": np.array(["2019-04-18", "2019-04-19"], dtype="datetime64"), "x": range(N), "y": range(N), - "x_nondim":("x",np.arange(N)) + "x_nondim": ("x", np.arange(N)), }, ) - + with pytest.raises(ValueError): xrft.power_spectrum(da) - - xrft.power_spectrum(da,dim=["time","y"]) + xrft.power_spectrum(da, dim=["time", "y"]) diff --git a/xrft/xrft.py b/xrft/xrft.py index e0144903..f7382827 100644 --- a/xrft/xrft.py +++ b/xrft/xrft.py @@ -385,7 +385,9 @@ def fft( # raise error if there are multiple coordinates attached to the dimension(s) over which the FFT is taken for d in dim: - bad_coords = [cname for cname in da.coords if cname != dim and dim in da[cname].dims] + bad_coords = [ + cname for cname in da.coords if cname != dim and dim in da[cname].dims + ] if bad_coords: raise ValueError( f"The input array contains coordinate variable(s) ({bad_coords}) whose dims include the transform dimension(s) `{dim}`. " From 9d04cba1b381d25a767c035c58b6caa39dff9d6b Mon Sep 17 00:00:00 2001 From: Paige Martin Date: Fri, 27 Aug 2021 12:04:05 +1000 Subject: [PATCH 6/6] fix typo in suggested change --- xrft/xrft.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xrft/xrft.py b/xrft/xrft.py index f7382827..1c38b6e0 100644 --- a/xrft/xrft.py +++ b/xrft/xrft.py @@ -386,11 +386,11 @@ def fft( # raise error if there are multiple coordinates attached to the dimension(s) over which the FFT is taken for d in dim: bad_coords = [ - cname for cname in da.coords if cname != dim and dim in da[cname].dims + cname for cname in da.coords if cname != d and d in da[cname].dims ] if bad_coords: raise ValueError( - f"The input array contains coordinate variable(s) ({bad_coords}) whose dims include the transform dimension(s) `{dim}`. " + f"The input array contains coordinate variable(s) ({bad_coords}) whose dims include the transform dimension(s) `{d}`. " f"Please drop these coordinates (`.drop({bad_coords}`) before invoking xrft." )