Silence some warnings. #2328

dcherian · 2018-07-29T01:46:27Z

Tests passed (for all non-documentation changes)

Remove some warnings.

dcherian · 2018-07-29T05:30:16Z

Most of the remaining warnings are:
RuntimeWarning: Cannot close a netcdf_file opened with mmap=True, when netcdf_variables or arrays referring to its data still exist. All data arrays obtained from such files refer directly to data on disk, and must be copied before the file can be cleanly closed. (See netcdf_file docstring for more information on mmap.)

fujiisoup

Thanks for the PR.
This is great.

Just a few comment regarding to isel_points and sel_points

fujiisoup · 2018-07-29T08:12:42Z

xarray/tests/test_dataarray.py

@@ -693,7 +693,7 @@ def test_isel_fancy(self):
        da.isel(time=(('points',), [1, 2]), x=(('points',), [2, 2]),
                y=(('points',), [3, 4]))
        np.testing.assert_allclose(
-            da.isel_points(time=[1], x=[2], y=[4]).values.squeeze(),
+            da.isel(time=[1], x=[2], y=[4]).values.squeeze(),


This does not give the same result.
I think we can just ignore the warning by @pytest.mark.filterwarnings.

fujiisoup · 2018-07-29T08:13:30Z

xarray/tests/test_dataset.py

@@ -1417,7 +1420,7 @@ def test_sel_fancy(self):
        assert_identical(actual['b'].drop('y'), idx_y['b'])

        with pytest.raises(KeyError):
-            data.sel_points(x=[2.5], y=[2.0], method='pad', tolerance=1e-3)
+            data.sel(x=[2.5], y=[2.0], method='pad', tolerance=1e-3)


Here also, we can just ignore the deprecation warning.

jhamman · 2018-07-29T16:14:09Z

Most of the remaining warnings are:
RuntimeWarning: Cannot close a netcdf_file opened with mmap=True, when netcdf_variables or arrays referring to its data still exist. All data arrays obtained from such files refer directly to data on disk, and must be copied before the file can be cleanly closed. (See netcdf_file docstring for more information on mmap.)

@dcherian - take a look here: #2261 (comment). These may be fixed soon.

Also, xrefing: #1652, #1657

shoyer

thanks!

shoyer · 2018-07-29T18:23:45Z

xarray/tests/test_backends.py

@@ -2519,6 +2520,7 @@ class PyNioTestAutocloseTrue(PyNioTest):


 @requires_pseudonetcdf
+@pytest.mark.filterwarnings('ignore:IOAPI_ISPH is assumed to be 6370000')


this is really nice! way better than using filterwarnings manually :)

shoyer · 2018-07-29T18:25:52Z

xarray/plot/plot.py

@@ -464,7 +461,7 @@ def _rescale_imshow_rgb(darray, vmin, vmax, robust):
    # After scaling, downcast to 32-bit float.  This substantially reduces
    # memory usage after we hand `darray` off to matplotlib.
    darray = ((darray.astype('f8') - vmin) / (vmax - vmin)).astype('f4')
-    return minimum(maximum(darray, 0), 1)
+    return np.minimum(np.maximum(darray, 0), 1)


__array_ufunc__ (which enalbes this on was added new in numpy 1.13. For now, we only require numpy 1.12, so let's stick with using xarray.ufuncs for now.

dcherian · 2018-07-29T19:48:44Z

@jhamman, @fujiisoup, @shoyer Thanks!

dcherian · 2018-07-29T19:50:14Z

xarray/core/missing.py

@@ -176,6 +176,15 @@ def get_clean_interp_index(arr, dim, use_coordinate=True, **kwargs):
        # raise if index cannot be cast to a float (e.g. MultiIndex)
        try:
            index = index.values.astype(np.float64)
+            if method != 'nearest':


rescaling is necessary to avoid underflow/overflow RuntimeWarnings with datetime64 (seen in tests). I've made a similar change to interp() below.

shoyer · 2018-07-29T22:15:31Z

xarray/core/missing.py

+                # Let's keep that compatitibility
+                index = (index - index.min())
+                if len(index) > 1:
+                    index /= index.std()


Standard deviation is probably fine here, but I wonder if it would be better to guarantee that are all values fall within some fixed range [-MAX, MAX], e.g., by using max() instead:

index = index - index.mean() index /= EPSILON * np.maximum(index.max(), 1)

where EPSILON is some constant, e.g., 1e-8.

shoyer · 2018-07-30T00:43:00Z

xarray/core/missing.py

                if len(index) > 1:
-                    index /= index.std()
+                    index /= np.max(np.abs(index))


This still needs a fix to account for possibly dividing by zero.

Also, I don't think you need abs() here because subtracting by mean() already centers the data.

The abs() makes sure that it's always within [-1, 1] right? e.g. if index = [-2, 0, 0.9] and as long as index is not all zeroes, it should always work? 🤔
Sorry, I feel like I'm missing something.

Yes, this would work fine unless the array is all zeros/constant.

It still might be worth considering that possibility, though at this point I'm pretty sure we are just refining total edge case behavior.

Maybe:

min_value = index.min() max_value = index.max() range_ = max_value - min_value if range_: midpoint = (min_value + max_value) / 2 index = index - midpoint index /= range_

The other option is to not really worry about this at all, besides silencing the warning. float64 is about the best we can do as far as precision, and it's a simple fact that you cannot represent every int64 value exactly in float64. In most cases this isn't a problem.

Arguably, this is the most predictable thing to do. I don't think it really helps with precision to rescale values in general, given that float64 can handle very large exponents just fine without any loss of precision.

I guess the only case where this would make a difference is if you really do care about nanosecond level precision and all your dates are within a single year or so. But that does seem a little unusual for xarray.

The people doing lightning research probably don't think that scenario is all that unusual (don't know for sure, I just remember a conversation with a lightning researcher when he complained about matplotlib's lack of support for nanosecond tickers).

The other option is to not really worry about this at all, besides silencing the warning. float64 is about the best we can do as far as precision, and it's a simple fact that you cannot represent every int64 value exactly in float64. In most cases this isn't a problem.

Ya, so the choice is between using filterwarnings or rescaling to avoid the warning. I have a mild preference for rescaling because it "solves" the problem. @shoyer: your call...

Hopefully, the lightning research people will chime in / send in a PR if this is actually an issue.

Let's just silence the warning here. I don't think rescaling actually fixes anything, and it's less efficient than silencing it.

These were being triggered by casting datetime64[ns] to float32. We now rescale the co-ordinate before interpolating, except for nearest-neighbour interpolation. The rescaling can change the nearest neighbour, and so is avoided in this case to maintain pandas compatibility.

This reverts commit 76f988f.

This reverts commit 9ac15ef.

This reverts commit 1f1ec52.

dcherian · 2018-08-20T17:54:07Z

xarray/tests/test_dataarray.py

@@ -3532,6 +3532,8 @@ def test_rolling_reduce(da, center, min_periods, window, name):
 @pytest.mark.parametrize('min_periods', (None, 1, 2, 3))
 @pytest.mark.parametrize('window', (1, 2, 3, 4))
 @pytest.mark.parametrize('name', ('sum', 'max'))
+@pytest.mark.filterwarnings('ignore:Using a non-tuple sequence')


Silences a warning that seems to be thrown by code in bottleneck

xarray/tests/test_dataarray.py::test_rolling_reduce_nonnumeric[sum-1-3-False] /home/travis/miniconda/envs/test_env/lib/python3.6/site-packages/bottleneck/slow/move.py:149: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result. nidx1 = n[idx1] /home/travis/miniconda/envs/test_env/lib/python3.6/site-packages/bottleneck/slow/move.py:150: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result. nidx1 = nidx1 - n[idx2] /home/travis/miniconda/envs/test_env/lib/python3.6/site-packages/bottleneck/slow/move.py:152: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result. idx[idx1] = nidx1 < min_count /home/travis/miniconda/envs/test_env/lib/python3.6/site-packages/bottleneck/slow/move.py:153: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result. idx[idx3] = n[idx3] < min_count

Good catch -- can you file a report in bottleneck? I expect this would be easy to fix.

OK. Filed a report upstream pydata/bottleneck#194 and reverted this commit.

This reverts commit b985127.

jhamman

LGTM. One small comment on dask version stuff but I'm excited to see this go in.

jhamman · 2018-09-04T04:17:19Z

xarray/tests/test_dask.py

-        with dask.set_options(get=dask.get):
+
+        with (dask.config.set(get=dask.get) if hasattr(dask, 'config')
+              else dask.set_options(get=dask.get)):


is this just a version check? Generally, I prefer to see a version comparison so we can more obviously clean these things up when older versions are no longer supported.

(Same comment below)

Ya it's basically a version check. I've made it an explicit version check now.

shoyer

Let’s merge this!

dcherian force-pushed the fix-some-warnings branch from 58218ff to 5f0e251 Compare July 29, 2018 02:22

fujiisoup reviewed Jul 29, 2018

View reviewed changes

shoyer reviewed Jul 29, 2018

View reviewed changes

dcherian commented Jul 29, 2018

View reviewed changes

dcherian changed the title ~~Fix some warnings.~~ Silence some warnings. Jul 29, 2018

shoyer reviewed Jul 29, 2018

View reviewed changes

shoyer reviewed Jul 30, 2018

View reviewed changes

dcherian added 13 commits August 20, 2018 17:35

Fix some warnings.

482ecf6

Make sure dask tests work with dask=0.16

6b46b15

Silence some pnetcdf warnings.

75cce6c

fix sel_points, isel_points fancy indexing tests

7518bd7

Revert to using xr.ufuncs

7186644

Rescale datetime for interp() too.

9ac15ef

Better rescaling.

76f988f

Revert "Better rescaling."

f605d6e

This reverts commit 76f988f.

Revert "Rescale datetime for interp() too."

9729f29

This reverts commit 9ac15ef.

Revert "Fix overflow/underflow warnings in interpolate_na"

cee7e2a

This reverts commit 1f1ec52.

Silence overflow/underflow/invalid value warnings.

fbdb206

Silence a bottleneck warning.

b985127

dcherian force-pushed the fix-some-warnings branch from 21c8c47 to b985127 Compare August 20, 2018 17:52

dcherian commented Aug 20, 2018

View reviewed changes

dcherian mentioned this pull request Sep 3, 2018

FutureWarning: Non-tuple sequence for multidimensional indexing pydata/bottleneck#194

Closed

Revert "Silence a bottleneck warning."

d9e8024

This reverts commit b985127.

jhamman reviewed Sep 4, 2018

View reviewed changes

shoyer approved these changes Sep 4, 2018

View reviewed changes

Dask: change from attribute check to version check.

2c0ed18

Maybe this fixes python 2 failure?

a74f4e0

dcherian merged commit a3ca579 into pydata:master Sep 4, 2018

dcherian deleted the fix-some-warnings branch September 4, 2018 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Silence some warnings. #2328

Silence some warnings. #2328

dcherian commented Jul 29, 2018

dcherian commented Jul 29, 2018

fujiisoup left a comment

fujiisoup Jul 29, 2018

fujiisoup Jul 29, 2018

jhamman commented Jul 29, 2018

shoyer left a comment

shoyer Jul 29, 2018

shoyer Jul 29, 2018

dcherian commented Jul 29, 2018

dcherian Jul 29, 2018 •

edited

Loading

shoyer Jul 29, 2018

shoyer Jul 30, 2018

dcherian Jul 30, 2018

shoyer Jul 30, 2018

shoyer Jul 30, 2018 •

edited

Loading

WeatherGod Jul 31, 2018

dcherian Aug 2, 2018 •

edited

Loading

shoyer Aug 19, 2018

dcherian Aug 20, 2018

shoyer Aug 20, 2018 •

edited

Loading

dcherian Sep 3, 2018

jhamman left a comment

jhamman Sep 4, 2018

dcherian Sep 4, 2018

shoyer left a comment

		@@ -2519,6 +2520,7 @@ class PyNioTestAutocloseTrue(PyNioTest):


		@requires_pseudonetcdf
		@pytest.mark.filterwarnings('ignore:IOAPI_ISPH is assumed to be 6370000')

Silence some warnings. #2328

Silence some warnings. #2328

Conversation

dcherian commented Jul 29, 2018

dcherian commented Jul 29, 2018

fujiisoup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhamman commented Jul 29, 2018

shoyer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcherian commented Jul 29, 2018

dcherian Jul 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer Jul 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcherian Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhamman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer left a comment

Choose a reason for hiding this comment

dcherian Jul 29, 2018 •

edited

Loading

shoyer Jul 30, 2018 •

edited

Loading

dcherian Aug 2, 2018 •

edited

Loading

shoyer Aug 20, 2018 •

edited

Loading