Repository: pydata/xarray

Issue:
Dimension Co-ordinates incorectly saving _FillValue attribute
#### Code Sample, a copy-pastable example if possible

```python
import xarray as xr
import pandas as pd
import numpy as np

temp = 273.15 + 25 * np.random.randn(2, 2)
lon = [0.0, 5.0]
lat = [10.0, 20.0]

ds = xr.Dataset({'temperature': (['lat', 'lon'], temp)},
                coords={'lat': lat,
                        'lon': lon})

ds['lat'].attrs = {
    'standard_name': 'latitude',
    'long_name': 'latitude',
    'units': 'degrees_north',
    'axis': 'Y'}
ds['lon'].attrs = {
    'standard_name': 'longitude',
    'long_name': 'longitude',
    'units': 'degrees_east',
    'axis': 'X'}
ds['temperature'].attrs = {
    'standard_name': 'air_temperature',
    'units': 'K'}
ds.attrs = {
    ('title', 'non-conforming CF 1.6 data produced by xarray 0.10'),
    ('Conventions', 'CF-1.6')}

ds.to_netcdf('/tmp/test.nc')

```
#### Problem description

According to the last sentence of the first paragraph of 2.5.1. *Missing data, valid and actual range of data* in NetCDF Climate and Forecast (CF) Metadata Conventions 1.7:

> Missing data is not allowed in coordinate variables.

When I use the [conformance checker](http://pumatest.nerc.ac.uk/cgi-bin/cf-checker-dev.pl?cfversion=auto) it issues an INFO message to this point for the co-ordinate variables.

Output of CF-Checker follows...

```
CHECKING` NetCDF FILE: /tmp/29428.nc
=====================
Using CF Checker Version 3.0.0
Checking against CF Version CF-1.6
Using Standard Name Table Version 48 (2017-11-28T15:32:48Z)
Using Area Type Table Version 6 (22 February 2017)


------------------
Checking variable: temperature
------------------

------------------
Checking variable: lat
------------------
INFO: attribute _FillValue is being used in a non-standard way

------------------
Checking variable: lon
------------------
INFO: attribute _FillValue is being used in a non-standard way

ERRORS detected: 0
WARNINGS given: 0
INFORMATION messages: 2
```

#### Expected Output

Co-ordinate variables should not store a _FillValue attribute

#### Output of ``xr.show_versions()``

INSTALLED VERSIONS
------------------
commit: None
python: 3.6.4.final.0
python-bits: 64
OS: Linux
OS-release: 4.14.15-1-ARCH
machine: x86_64
processor: 
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8
xarray: 0.10.0
pandas: 0.22.0
numpy: 1.14.0
scipy: 1.0.0
netCDF4: 1.3.1
h5netcdf: 0.5.0
Nio: None
bottleneck: 1.2.1
cyordereddict: None
dask: 0.16.1
matplotlib: 2.1.2
cartopy: None
seaborn: None
setuptools: 38.4.0
pip: 9.0.1
conda: None
pytest: None
IPython: None
sphinx: None


</details>


Discussion:
shoyer: Oops! Yes, this is was definitely a bug.

Can you add a unit test for this, so we don't have a regression?

Take a look at the test `test_explicitly_omit_fill_value ` added in https://github.com/pydata/xarray/pull/1659 and `test_encoding_same_dtype` below it for examples.
czr137: Unit tests added in 9210dfa45428842c69b8d1835013159b1230c572. The first and the third added tests use the encoding kwarg and fail without b8fd86d5614d28254406d6f9b6d4e37383efc1be applied and pass with b8fd86d5614d28254406d6f9b6d4e37383efc1be applied, so they should catch a regression.

The second added test passes both before and after b8fd86d5614d28254406d6f9b6d4e37383efc1be and was added for completeness as it pairs with the original test_explicitly_omit_fill_value, but for a coordinate.

I still think nothing needs to change in the documentation as this now follows the intent of the design.

I will leave it to you @shoyer or your delegate to address whether anything needs to be stated in the changelog / what's new.
czr137: Wait to merge... I'm still getting failed tests using the scipy backend
czr137: Now the tests pass (see comment beside the checkbox)

Honestly, I'm not happy with where I put the fix in order to get the tests to pass when using the scipy backend by adding an additional test, but I didn't know where else to put the test. The test to see if the variable would be checked in prepare_variables came before the encoding could be managed in encode_cf_variable in conventions.py
czr137: @shoyer, do you have an opinion on whether or not to include the scipy fix or leave it out of this PR?
Also what is your opinion on the documentation? I feel nothing is needed as with this PR the code will operate as the documentation states.
shoyer: We don't need any additional documentation, but we should note this bug fix in "What's new"

Patch:
diff --git a/doc/gallery/plot_rasterio_rgb.py b/doc/gallery/plot_rasterio_rgb.py
index 35c1d044..ec2bbe63 100644
--- a/doc/gallery/plot_rasterio_rgb.py
+++ b/doc/gallery/plot_rasterio_rgb.py
@@ -26,9 +26,6 @@ urllib.request.urlretrieve(url, 'RGB.byte.tif')
 # Read the data
 da = xr.open_rasterio('RGB.byte.tif')
 
-# Normalize the image
-da = da / 255
-
 # The data is in UTM projection. We have to set it manually until
 # https://github.com/SciTools/cartopy/issues/813 is implemented
 crs = ccrs.UTM('18N')
diff --git a/doc/time-series.rst b/doc/time-series.rst
index bdf8b1e7..4d9a9950 100644
--- a/doc/time-series.rst
+++ b/doc/time-series.rst
@@ -129,6 +129,14 @@ the first letters of the corresponding months.
 
 You can use these shortcuts with both Datasets and DataArray coordinates.
 
+In addition, xarray supports rounding operations ``floor``, ``ceil``, and ``round``. These operations require that you supply a `rounding frequency as a string argument.`__
+
+__ http://pandas.pydata.org/pandas-docs/stable/timeseries.html#offset-aliases
+
+.. ipython:: python
+
+    ds['time'].dt.floor('D')
+
 .. _resampling:
 
 Resampling and grouped operations
diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index f17bb8f0..875aba51 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -79,6 +79,8 @@ Enhancements
   (:pull:`1840`), and keeping float16 and float32 as float32 (:issue:`1842`).
   Correspondingly, encoded variables may also be saved with a smaller dtype.
   By `Zac Hatfield-Dodds <https://github.com/Zac-HD>`_.
+- `.dt` accessor can now ceil, floor and round timestamps to specified frequency.
+  By `Deepak Cherian <https://github.com/dcherian>`_.
 
 .. _Zarr: http://zarr.readthedocs.io/
 
@@ -94,7 +96,7 @@ Bug fixes
 ~~~~~~~~~
 - Added warning in api.py of a netCDF4 bug that occurs when
   the filepath has 88 characters (:issue:`1745`).
-  By `Liam Brannigan <https://github.com/braaannigan>` _.
+  By `Liam Brannigan <https://github.com/braaannigan>`_.
 - Fixed encoding of multi-dimensional coordinates in
   :py:meth:`~Dataset.to_netcdf` (:issue:`1763`).
   By `Mike Neish <https://github.com/neishm>`_.
@@ -126,6 +128,9 @@ Bug fixes
 - Compatibility fixes to plotting module for Numpy 1.14 and Pandas 0.22
   (:issue:`1813`).
   By `Joe Hamman <https://github.com/jhamman>`_.
+- Bug fix in encoding coordinates with ``{'_FillValue': None}`` in netCDF
+  metadata (:issue:`1865`).
+  By `Chris Roth <https://github.com/czr137>`_.
 - Fix indexing with lists for arrays loaded from netCDF files with
   ``engine='h5netcdf`` (:issue:`1864`).
   By `Stephan Hoyer <https://github.com/shoyer>`_.
diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py
index b3cb1d8e..3f3364de 100644
--- a/xarray/backends/netCDF4_.py
+++ b/xarray/backends/netCDF4_.py
@@ -160,7 +160,7 @@ def _extract_nc4_variable_encoding(variable, raise_on_invalid=False,
 
     safe_to_drop = set(['source', 'original_shape'])
     valid_encodings = set(['zlib', 'complevel', 'fletcher32', 'contiguous',
-                           'chunksizes', 'shuffle'])
+                           'chunksizes', 'shuffle', '_FillValue'])
     if lsd_okay:
         valid_encodings.add('least_significant_digit')
 
diff --git a/xarray/backends/scipy_.py b/xarray/backends/scipy_.py
index dba2e567..a608cff8 100644
--- a/xarray/backends/scipy_.py
+++ b/xarray/backends/scipy_.py
@@ -188,8 +188,9 @@ class ScipyDataStore(WritableCFDataStore, DataStorePickleMixin):
     def prepare_variable(self, name, variable, check_encoding=False,
                          unlimited_dims=None):
         if check_encoding and variable.encoding:
-            raise ValueError('unexpected encoding for scipy backend: %r'
-                             % list(variable.encoding))
+            if variable.encoding != {'_FillValue': None}:
+                raise ValueError('unexpected encoding for scipy backend: %r'
+                                 % list(variable.encoding))
 
         data = variable.data
         # nb. this still creates a numpy array in all memory, even though we
diff --git a/xarray/core/accessors.py b/xarray/core/accessors.py
index 5052b555..b3e7e1ff 100644
--- a/xarray/core/accessors.py
+++ b/xarray/core/accessors.py
@@ -58,6 +58,43 @@ def _get_date_field(values, name, dtype):
         return _access_through_series(values, name)
 
 
+def _round_series(values, name, freq):
+    """Coerce an array of datetime-like values to a pandas Series and
+    apply requested rounding
+    """
+    values_as_series = pd.Series(values.ravel())
+    method = getattr(values_as_series.dt, name)
+    field_values = method(freq=freq).values
+
+    return field_values.reshape(values.shape)
+
+
+def _round_field(values, name, freq):
+    """Indirectly access pandas rounding functions by wrapping data
+    as a Series and calling through `.dt` attribute.
+
+    Parameters
+    ----------
+    values : np.ndarray or dask.array-like
+        Array-like container of datetime-like values
+    name : str (ceil, floor, round)
+        Name of rounding function
+    freq : a freq string indicating the rounding resolution
+
+    Returns
+    -------
+    rounded timestamps : same type as values
+        Array-like of datetime fields accessed for each element in values
+
+    """
+    if isinstance(values, dask_array_type):
+        from dask.array import map_blocks
+        return map_blocks(_round_series,
+                          values, name, freq=freq, dtype=np.datetime64)
+    else:
+        return _round_series(values, name, freq)
+
+
 class DatetimeAccessor(object):
     """Access datetime fields for DataArrays with datetime-like dtypes.
 
@@ -147,3 +184,58 @@ class DatetimeAccessor(object):
     time = _tslib_field_accessor(
         "time", "Timestamps corresponding to datetimes", object
     )
+
+    def _tslib_round_accessor(self, name, freq):
+        obj_type = type(self._obj)
+        result = _round_field(self._obj.data, name, freq)
+        return obj_type(result, name=name,
+                        coords=self._obj.coords, dims=self._obj.dims)
+
+    def floor(self, freq):
+        '''
+        Round timestamps downward to specified frequency resolution.
+
+        Parameters
+        ----------
+        freq : a freq string indicating the rounding resolution
+            e.g. 'D' for daily resolution
+
+        Returns
+        -------
+        floor-ed timestamps : same type as values
+            Array-like of datetime fields accessed for each element in values
+        '''
+
+        return self._tslib_round_accessor("floor", freq)
+
+    def ceil(self, freq):
+        '''
+        Round timestamps upward to specified frequency resolution.
+
+        Parameters
+        ----------
+        freq : a freq string indicating the rounding resolution
+            e.g. 'D' for daily resolution
+
+        Returns
+        -------
+        ceil-ed timestamps : same type as values
+            Array-like of datetime fields accessed for each element in values
+        '''
+        return self._tslib_round_accessor("ceil", freq)
+
+    def round(self, freq):
+        '''
+        Round timestamps to specified frequency resolution.
+
+        Parameters
+        ----------
+        freq : a freq string indicating the rounding resolution
+            e.g. 'D' for daily resolution
+
+        Returns
+        -------
+        rounded timestamps : same type as values
+            Array-like of datetime fields accessed for each element in values
+        '''
+        return self._tslib_round_accessor("round", freq)
diff --git a/xarray/plot/plot.py b/xarray/plot/plot.py
index d17ceb84..97fee5f0 100644
--- a/xarray/plot/plot.py
+++ b/xarray/plot/plot.py
@@ -710,8 +710,12 @@ def imshow(x, y, z, ax, **kwargs):
         # missing data transparent.  We therefore add an alpha channel if
         # there isn't one, and set it to transparent where data is masked.
         if z.shape[-1] == 3:
-            z = np.ma.concatenate((z, np.ma.ones(z.shape[:2] + (1,))), 2)
-        z = z.copy()
+            alpha = np.ma.ones(z.shape[:2] + (1,), dtype=z.dtype)
+            if np.issubdtype(z.dtype, np.integer):
+                alpha *= 255
+            z = np.ma.concatenate((z, alpha), axis=2)
+        else:
+            z = z.copy()
         z[np.any(z.mask, axis=-1), -1] = 0
 
     primitive = ax.imshow(z, **defaults)


Test patch:
diff --git a/xarray/tests/test_accessors.py b/xarray/tests/test_accessors.py
index 30ea1a88..1fcde8f5 100644
--- a/xarray/tests/test_accessors.py
+++ b/xarray/tests/test_accessors.py
@@ -57,6 +57,9 @@ class TestDatetimeAccessor(TestCase):
         months = self.times_data.dt.month
         hours = self.times_data.dt.hour
         days = self.times_data.dt.day
+        floor = self.times_data.dt.floor('D')
+        ceil = self.times_data.dt.ceil('D')
+        round = self.times_data.dt.round('D')
 
         dask_times_arr = da.from_array(self.times_arr, chunks=(5, 5, 50))
         dask_times_2d = xr.DataArray(dask_times_arr,
@@ -67,6 +70,9 @@ class TestDatetimeAccessor(TestCase):
         dask_month = dask_times_2d.dt.month
         dask_day = dask_times_2d.dt.day
         dask_hour = dask_times_2d.dt.hour
+        dask_floor = dask_times_2d.dt.floor('D')
+        dask_ceil = dask_times_2d.dt.ceil('D')
+        dask_round = dask_times_2d.dt.round('D')
 
         # Test that the data isn't eagerly evaluated
         assert isinstance(dask_year.data, da.Array)
@@ -86,6 +92,9 @@ class TestDatetimeAccessor(TestCase):
         assert_equal(months, dask_month.compute())
         assert_equal(days, dask_day.compute())
         assert_equal(hours, dask_hour.compute())
+        assert_equal(floor, dask_floor.compute())
+        assert_equal(ceil, dask_ceil.compute())
+        assert_equal(round, dask_round.compute())
 
     def test_seasons(self):
         dates = pd.date_range(start="2000/01/01", freq="M", periods=12)
@@ -95,3 +104,14 @@ class TestDatetimeAccessor(TestCase):
         seasons = xr.DataArray(seasons)
 
         assert_array_equal(seasons.values, dates.dt.season.values)
+
+    def test_rounders(self):
+        dates = pd.date_range("2014-01-01", "2014-05-01", freq='H')
+        xdates = xr.DataArray(np.arange(len(dates)),
+                              dims=['time'], coords=[dates])
+        assert_array_equal(dates.floor('D').values,
+                           xdates.time.dt.floor('D').values)
+        assert_array_equal(dates.ceil('D').values,
+                           xdates.time.dt.ceil('D').values)
+        assert_array_equal(dates.round('D').values,
+                           xdates.time.dt.round('D').values)
diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py
index 85b6bdea..e88fc790 100644
--- a/xarray/tests/test_backends.py
+++ b/xarray/tests/test_backends.py
@@ -704,6 +704,26 @@ class CFEncodedDataTest(DatasetIOTestCases):
         with self.roundtrip(ds) as actual:
             assert '_FillValue' not in actual.x.encoding
 
+    def test_explicitly_omit_fill_value_via_encoding_kwarg(self):
+        ds = Dataset({'x': ('y', [np.pi, -np.pi])})
+        kwargs = dict(encoding={'x': {'_FillValue': None}})
+        with self.roundtrip(ds, save_kwargs=kwargs) as actual:
+            assert '_FillValue' not in actual.x.encoding
+        self.assertEqual(ds.y.encoding, {})
+
+    def test_explicitly_omit_fill_value_in_coord(self):
+        ds = Dataset({'x': ('y', [np.pi, -np.pi])}, coords={'y': [0.0, 1.0]})
+        ds.y.encoding['_FillValue'] = None
+        with self.roundtrip(ds) as actual:
+            assert '_FillValue' not in actual.y.encoding
+
+    def test_explicitly_omit_fill_value_in_coord_via_encoding_kwarg(self):
+        ds = Dataset({'x': ('y', [np.pi, -np.pi])}, coords={'y': [0.0, 1.0]})
+        kwargs = dict(encoding={'y': {'_FillValue': None}})
+        with self.roundtrip(ds, save_kwargs=kwargs) as actual:
+            assert '_FillValue' not in actual.y.encoding
+        self.assertEqual(ds.y.encoding, {})
+
     def test_encoding_same_dtype(self):
         ds = Dataset({'x': ('y', np.arange(10.0, dtype='f4'))})
         kwargs = dict(encoding={'x': {'dtype': 'f4'}})
diff --git a/xarray/tests/test_distributed.py b/xarray/tests/test_distributed.py
index 1d450ff5..47bb6cdc 100644
--- a/xarray/tests/test_distributed.py
+++ b/xarray/tests/test_distributed.py
@@ -81,4 +81,4 @@ def test_async(c, s, a, b):
     assert not dask.is_dask_collection(w)
     assert_allclose(x + 10, w)
 
-    assert s.task_state
+    assert s.tasks
diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py
index 1573577a..479b1dce 100644
--- a/xarray/tests/test_plot.py
+++ b/xarray/tests/test_plot.py
@@ -1146,6 +1146,13 @@ class TestImshow(Common2dMixin, PlotTestCase):
         for kwds in [dict(vmax=-1, vmin=-1.2), dict(vmin=2, vmax=2.1)]:
             da.plot.imshow(**kwds)
 
+    def test_imshow_rgb_values_in_valid_range(self):
+        da = DataArray(np.arange(75, dtype='uint8').reshape((5, 5, 3)))
+        _, ax = plt.subplots()
+        out = da.plot.imshow(ax=ax).get_array()
+        assert out.dtype == np.uint8
+        assert (out[..., :3] == da.values).all()  # Compare without added alpha
+
 
 class TestFacetGrid(PlotTestCase):
     def setUp(self):