Repository: pydata/xarray

Issue:
_FillValue and missing_value not allowed for the same variable
#### Problem description

Currently opening the file which has a variable with both _FillValue and missing_value defined, results in a ValueError "'Discovered conflicting _FillValue and missing_value...", which can be overridden by setting `decode_cf=False` 

Code in `xarray/conventions.py` have comments that `missing_value is deprecated`, however this seems not to be true anymore, see [ticket #67](https://cf-trac.llnl.gov/trac/ticket/67) in http://cfconventions.org/cf-conventions/cf-conventions.html

Also, see description of meanings of _FillValue and missing_value here: 
https://github.com/Unidata/netcdf4-python/issues/94
"Note that missing_value and _FillValue are very different. _FillValue is used by the C library to fill in values in variables that have not yet been written to. missing_value is meant to represent values set by the user to represent data points that do not exist."

#### Expected Output
Accept both _FillValue and missing_value and if both present, mask output only for _FillValue

#### Output of ``xr.show_versions()``
0.9.6



Discussion:
dopplershift: Test failures look unrelated--they also happen to me locally. Installing SciPy 1.0.0 locally fixes them, so I'm guessing they're caused by SciPy 1.0.1.
shoyer: please merge in master, which fixes the failing tests.
dopplershift: Rebased on current master.
dopplershift: Fixed flake8 (unnecessary import)
dopplershift: Reworked due to corner case with PyNio on Python 2.7. Final solution looks simpler to me.

Patch:
diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index b28beb9e..eb7d33c9 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -40,6 +40,10 @@ Enhancements
  - Some speed improvement to construct :py:class:`~xarray.DataArrayRolling`
  object (:issue:`1993`)
  By `Keisuke Fujii <https://github.com/fujiisoup>`_.
+ - Handle variables with different values for ``missing_value`` and
+ ``_FillValue`` by masking values for both attributes; previously this
+ resulted in a ``ValueError``. (:issue:`2016`)
+ By `Ryan May <https://github.com/dopplershift>`_.
 
 Bug fixes
 ~~~~~~~~~
diff --git a/xarray/backends/netCDF4_.py b/xarray/backends/netCDF4_.py
index 28aa4dbd..89a0e72e 100644
--- a/xarray/backends/netCDF4_.py
+++ b/xarray/backends/netCDF4_.py
@@ -264,13 +264,13 @@ class NetCDF4DataStore(WritableCFDataStore, DataStorePickleMixin):
         if (len(filename) == 88 and
                 LooseVersion(nc4.__version__) < "1.3.1"):
             warnings.warn(
-                '\nA segmentation fault may occur when the\n'
-                'file path has exactly 88 characters as it does\n'
-                'in this case. The issue is known to occur with\n'
-                'version 1.2.4 of netCDF4 and can be addressed by\n'
-                'upgrading netCDF4 to at least version 1.3.1.\n'
-                'More details can be found here:\n'
-                'https://github.com/pydata/xarray/issues/1745  \n')
+                'A segmentation fault may occur when the '
+                'file path has exactly 88 characters as it does '
+                'in this case. The issue is known to occur with '
+                'version 1.2.4 of netCDF4 and can be addressed by '
+                'upgrading netCDF4 to at least version 1.3.1. '
+                'More details can be found here: '
+                'https://github.com/pydata/xarray/issues/1745')
         if format is None:
             format = 'NETCDF4'
         opener = functools.partial(_open_netcdf4_group, filename, mode=mode,
diff --git a/xarray/coding/variables.py b/xarray/coding/variables.py
index ced53564..4e61e7d4 100644
--- a/xarray/coding/variables.py
+++ b/xarray/coding/variables.py
@@ -7,7 +7,7 @@ from functools import partial
 import numpy as np
 import pandas as pd
 
-from ..core import dtypes, duck_array_ops, indexing, utils
+from ..core import dtypes, duck_array_ops, indexing
 from ..core.pycompat import dask_array_type
 from ..core.variable import Variable
 
@@ -152,26 +152,12 @@ class CFMaskCoder(VariableCoder):
     def decode(self, variable, name=None):
         dims, data, attrs, encoding = unpack_for_decoding(variable)
 
-        if 'missing_value' in attrs:
-            # missing_value is deprecated, but we still want to support it as
-            # an alias for _FillValue.
-            if ('_FillValue' in attrs and
-                not utils.equivalent(attrs['_FillValue'],
-                                     attrs['missing_value'])):
-                raise ValueError("Conflicting _FillValue and missing_value "
-                                 "attrs on a variable {!r}: {} vs. {}\n\n"
-                                 "Consider opening the offending dataset "
-                                 "using decode_cf=False, correcting the "
-                                 "attrs and decoding explicitly using "
-                                 "xarray.decode_cf()."
-                                 .format(name, attrs['_FillValue'],
-                                         attrs['missing_value']))
-            attrs['_FillValue'] = attrs.pop('missing_value')
-
-        if '_FillValue' in attrs:
-            raw_fill_value = pop_to(attrs, encoding, '_FillValue', name=name)
-            encoded_fill_values = [
-                fv for fv in np.ravel(raw_fill_value) if not pd.isnull(fv)]
+        raw_fill_values = [pop_to(attrs, encoding, attr, name=name)
+                           for attr in ('missing_value', '_FillValue')]
+        if raw_fill_values:
+            encoded_fill_values = {fv for option in raw_fill_values
+                                   for fv in np.ravel(option)
+                                   if not pd.isnull(fv)}
 
             if len(encoded_fill_values) > 1:
                 warnings.warn("variable {!r} has multiple fill values {}, "


Test patch:
diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py
index 0bef13a8..e0f03036 100644
--- a/xarray/tests/test_backends.py
+++ b/xarray/tests/test_backends.py
@@ -1137,8 +1137,10 @@ class NetCDF4DataTest(BaseNetCDF4Test, TestCase):
         # should be fixed in netcdf4 v1.3.1
         with mock.patch('netCDF4.__version__', '1.2.4'):
             with warnings.catch_warnings():
-                warnings.simplefilter("error")
-                with raises_regex(Warning, 'segmentation fault'):
+                message = ('A segmentation fault may occur when the '
+                           'file path has exactly 88 characters')
+                warnings.filterwarnings('error', message)
+                with pytest.raises(Warning):
                     # Need to construct 88 character filepath
                     xr.Dataset().to_netcdf('a' * (88 - len(os.getcwd()) - 1))
 
diff --git a/xarray/tests/test_conventions.py b/xarray/tests/test_conventions.py
index 7028bac7..5b82f5a2 100644
--- a/xarray/tests/test_conventions.py
+++ b/xarray/tests/test_conventions.py
@@ -156,12 +156,15 @@ class TestNativeEndiannessArray(TestCase):
 
 
 def test_decode_cf_with_conflicting_fill_missing_value():
-    var = Variable(['t'], np.arange(10),
+    expected = Variable(['t'], [np.nan, np.nan, 2], {'units': 'foobar'})
+    var = Variable(['t'], np.arange(3),
                    {'units': 'foobar',
                     'missing_value': 0,
                     '_FillValue': 1})
-    with raises_regex(ValueError, "_FillValue and missing_value"):
-        conventions.decode_cf_variable('t', var)
+    with warnings.catch_warnings(record=True) as w:
+        actual = conventions.decode_cf_variable('t', var)
+        assert_identical(actual, expected)
+        assert 'has multiple fill' in str(w[0].message)
 
     expected = Variable(['t'], np.arange(10), {'units': 'foobar'})