Updates to getitem (#196)

Author: dcherian

cf_xarray/accessor.py

@@ -12,7 +12,6 @@
     List,
     Mapping,
     MutableMapping,
-    Optional,
     Set,
     Tuple,
     TypeVar,
@@ -605,24 +604,41 @@ def _getitem(
     if skip is None:
         skip = []
 
-    def check_results(names, k):
+    def drop_bounds(names):
+        # sometimes bounds variables have the same standard_name as the
+        # actual variable. It seems practical to ignore them when indexing
+        # with a scalar key. Hopefully these will soon get decoded to IntervalIndex
+        # and we can move on...
+        if scalar_key:
+            bounds = set([obj[k].attrs.get("bounds", None) for k in names])
+            names = set(names) - bounds
+        return names
+
+    def check_results(names, key):
         if scalar_key and len(names) > 1:
             raise KeyError(
-                f"Receive multiple variables for key {k!r}: {names}. "
-                f"Expected only one. Please pass a list [{k!r}] "
-                f"instead to get all variables matching {k!r}."
+                f"Receive multiple variables for key {key!r}: {names}. "
+                f"Expected only one. Please pass a list [{key!r}] "
+                f"instead to get all variables matching {key!r}."
             )
 
+    try:
+        measures = accessor._get_all_cell_measures()
+    except ValueError:
+        measures = []
+        warnings.warn("Ignoring bad cell_measures attribute.", UserWarning)
+
     varnames: List[Hashable] = []
     coords: List[Hashable] = []
     successful = dict.fromkeys(key, False)
     for k in key:
         if "coords" not in skip and k in _AXIS_NAMES + _COORD_NAMES:
             names = _get_all(obj, k)
+            names = drop_bounds(names)
             check_results(names, k)
             successful[k] = bool(names)
             coords.extend(names)
-        elif "measures" not in skip and k in accessor._get_all_cell_measures():
+        elif "measures" not in skip and k in measures:
             measure = _get_all(obj, k)
             check_results(measure, k)
             successful[k] = bool(measure)
@@ -631,6 +647,7 @@ def check_results(names, k):
         else:
             stdnames = set(_get_with_standard_name(obj, k))
             objcoords = set(obj.coords)
+            stdnames = drop_bounds(stdnames)
             if "coords" in skip:
                 stdnames -= objcoords
             check_results(stdnames, k)
@@ -646,7 +663,7 @@ def check_results(names, k):
     try:
         for name in allnames:
             extravars = accessor.get_associated_variable_names(
-                name, skip_bounds=scalar_key
+                name, skip_bounds=scalar_key, error=False
             )
             coords.extend(itertools.chain(*extravars.values()))
 
@@ -1238,7 +1255,7 @@ def standard_names(self) -> Dict[str, List[str]]:
         return {k: sorted(v) for k, v in vardict.items()}
 
     def get_associated_variable_names(
-        self, name: Hashable, skip_bounds: Optional[bool] = None
+        self, name: Hashable, skip_bounds: bool = False, error: bool = True
     ) -> Dict[str, List[str]]:
         """
         Returns a dict mapping
@@ -1252,6 +1269,9 @@ def get_associated_variable_names(
         ----------
         name : Hashable
         skip_bounds : bool, optional
+        error: bool, optional
+            Raise or ignore errors.
+
         Returns
         ------
         Dict with keys "ancillary_variables", "cell_measures", "coordinates", "bounds"
@@ -1264,9 +1284,20 @@ def get_associated_variable_names(
             coords["coordinates"] = attrs_or_encoding["coordinates"].split(" ")
 
         if "cell_measures" in attrs_or_encoding:
-            coords["cell_measures"] = list(
-                parse_cell_methods_attr(attrs_or_encoding["cell_measures"]).values()
-            )
+            try:
+                coords["cell_measures"] = list(
+                    parse_cell_methods_attr(attrs_or_encoding["cell_measures"]).values()
+                )
+            except ValueError as e:
+                if error:
+                    msg = e.args[0] + " Ignore this error by passing 'error=False'"
+                    raise ValueError(msg)
+                else:
+                    warnings.warn(
+                        f"Ignoring bad cell_measures attribute: {attrs_or_encoding['cell_measures']}",
+                        UserWarning,
+                    )
+                    coords["cell_measures"] = []
 
         if (
             isinstance(self._obj, Dataset)

cf_xarray/tests/test_accessor.py

@@ -502,11 +502,51 @@ def test_getitem_errors(obj):
         obj2.cf["X"]
 
 
-def test_getitem_regression():
+def test_getitem_ignores_bad_measure_attribute():
+    air2 = airds.copy(deep=True)
+    air2.air.attrs["cell_measures"] = "asd"
+    with pytest.warns(UserWarning):
+        assert_identical(air2.air.drop_vars("cell_area"), air2.cf["air"])
+
+    with pytest.raises(ValueError):
+        air2.cf.cell_measures
+    with pytest.raises(ValueError):
+        air2.air.cf.cell_measures
+    with pytest.raises(ValueError):
+        air2.cf.get_associated_variable_names("air", error=True)
+    with pytest.warns(UserWarning):
+        air2.cf.get_associated_variable_names("air", error=False)
+
+
+def test_getitem_clash_standard_name():
     ds = xr.Dataset()
     ds.coords["area"] = xr.DataArray(np.ones(10), attrs={"standard_name": "cell_area"})
     assert_identical(ds.cf["cell_area"], ds["area"].reset_coords(drop=True))
 
+    ds = xr.Dataset()
+    ds["time"] = (
+        "time",
+        np.arange(10),
+        {"standard_name": "time", "bounds": "time_bounds"},
+    )
+    ds["time_bounds"] = (
+        ("time", "bounds"),
+        np.ones((10, 2)),
+        {"standard_name": "time"},
+    )
+
+    ds["lat"] = (
+        "lat",
+        np.arange(10),
+        {"units": "degrees_north", "bounds": "lat_bounds"},
+    )
+    ds["lat_bounds"] = (
+        ("lat", "bounds"),
+        np.ones((10, 2)),
+        {"units": "degrees_north"},
+    )
+    assert_identical(ds["lat"], ds.cf["latitude"])
+
 
 def test_getitem_uses_coordinates():
     # POP-like dataset