Repository: euro-cordex/py-cordex

Issue:
add `bounds` attribute for `lon`, `lat` coordinates

Discussion:
codecov-commenter: # [Codecov](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex) Report
> Merging [#101](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex) (a5a44b7) into [master](https://codecov.io/gh/euro-cordex/py-cordex/commit/d444a09d516f1cd1884af727198dd547b20e1ed7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex) (d444a09) will **decrease** coverage by `2.85%`.
> The diff coverage is `66.66%`.

> :exclamation: Current head a5a44b7 differs from pull request most recent head c629834. Consider uploading reports for the commit c629834 to get more accurate results

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex)

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://codecov.io/gh/euro-cordex/py-cordex/pull/101/graphs/tree.svg?width=650&height=150&src=pr&token=D6BAMOZZT3&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex)](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex)

```diff
@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
- Coverage   77.14%   74.29%   -2.85%     
==========================================
  Files          22       22              
  Lines        1098     1097       -1     
==========================================
- Hits          847      815      -32     
- Misses        251      282      +31     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `74.29% <66.66%> (-2.85%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex) | Coverage Δ | |
|---|---|---|
| [cordex/domain.py](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex#diff-Y29yZGV4L2RvbWFpbi5weQ==) | `75.00% <66.66%> (+1.99%)` | :arrow_up: |
| [cordex/preprocessing/preprocessing.py](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex#diff-Y29yZGV4L3ByZXByb2Nlc3NpbmcvcHJlcHJvY2Vzc2luZy5weQ==) | `63.03% <0.00%> (-17.07%)` | :arrow_down: |
| [cordex/cmor/cmor.py](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex#diff-Y29yZGV4L2Ntb3IvY21vci5weQ==) | `64.68% <0.00%> (+0.23%)` | :arrow_up: |
| [cordex/cmor/utils.py](https://codecov.io/gh/euro-cordex/py-cordex/pull/101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=euro-cordex#diff-Y29yZGV4L2Ntb3IvdXRpbHMucHk=) | `92.07% <0.00%> (+0.90%)` | :arrow_up: |

</details>

Patch:
diff --git a/cordex/cf.py b/cordex/cf.py
index a9b12c5..5cd7c76 100644
--- a/cordex/cf.py
+++ b/cordex/cf.py
@@ -84,3 +84,5 @@ DEFAULT_CORDEX_ATTRS = {
     "realization": 0,
     "cmor_version": "",
 }
+
+grid_mapping_dtype = np.int32
diff --git a/cordex/domain.py b/cordex/domain.py
index fdf4510..0982a78 100644
--- a/cordex/domain.py
+++ b/cordex/domain.py
@@ -59,7 +59,7 @@ def cordex_domain(
     tables=None,
     attrs=None,
     mapping_name=None,
-    bounds=None,
+    bounds=False,
 ):
     """Creates an xarray dataset containg the domain grid definitions.
 
@@ -73,6 +73,12 @@ def cordex_domain(
         looking at the domain with ncview.
     add_vertices : bool
         Add grid boundaries in the gloabl coordinates (lon_vertices and lat_vertices).
+
+        .. deprecated:: v0.5.0
+            The ``add_vertices`` parameter is deprecated in favor
+            of the ``bounds`` parameter, and will be removed in a future
+            version.
+
     tables: dataframe or list of dataframes, default: cordex_tables
         Tables from which to look up the grid information. Index in the table
         should be the short name of the domain, e.g., `EUR-11`. If no table is
@@ -83,13 +89,15 @@ def cordex_domain(
     mapping_name: str
         Variable name of the grid mapping, if mapping_name is `None`, the CF standard
         variable name is used.
-    bounds: str
-        Add spatial bounds.
+    bounds: bool
+        Add spatial bounds to longitude and latitude coordinates.
+
+        .. versionadded:: v0.5.0
 
     Returns
     -------
-    Dataset : xarray.core.Dataset
-        Dataset containing the coordinates.
+    Grid : xr.Dataset
+        Dataset containing a CORDEX grid.
 
     Example
     -------
@@ -101,6 +109,14 @@ def cordex_domain(
         eur11 = cx.cordex_domain('EUR-11')
 
     """
+    if add_vertices is True:
+        warn(
+            "add_vertices keyword is deprecated, please use the bounds keyword instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        bounds = True
+
     if attrs is None:
         attrs = {}
     if tables is None:
@@ -111,9 +127,9 @@ def cordex_domain(
         config = tables.loc[short_name]
     return create_dataset(
         **config,
-        name=short_name,
+        short_name=short_name,
         dummy=dummy,
-        add_vertices=add_vertices,
+        add_vertices=False,
         attrs=attrs,
         mapping_name=mapping_name,
         bounds=bounds
@@ -130,11 +146,12 @@ def create_dataset(
     pollon=None,
     pollat=None,
     name=None,
+    short_name=None,
     dummy=False,
     add_vertices=False,
     attrs=None,
     mapping_name=None,
-    bounds=None,
+    bounds=False,
     **kwargs
 ):
     """Create domain dataset from grid information.
@@ -157,19 +174,45 @@ def create_dataset(
         pol longitude (degrees)
     pollat : float
         pol latitude (degrees)
+    short_name : str
+        CORDEX domain identifier, goes into the ``CORDEX_domain`` global attribute.
     dummy : str or logical
         Name of dummy field, if dummy=topo, the cdo topo operator will be
         used to create some dummy topography data. dummy data is useful for
         looking at the domain with ncview.
     add_vertices : bool
         Add grid boundaries in the global coordinates (lon_vertices and lat_vertices).
+
+        .. deprecated:: v0.5.0
+            The ``add_vertices`` parameter is deprecated in favor
+            of the ``bounds`` parameter, and will be removed in a future
+            version.
+
     attrs: str or dict
         Global attributes that should be added to the dataset. If `attrs='CORDEX'`
         a set of standard CF global attributes.
     mapping_name: str
         Variable name of the grid mapping, if mapping_name is `None`, the CF standard
         variable name is used.
+    bounds: bool
+        Add spatial bounds to longitude and latitude coordinates.
+
+        .. versionadded:: v0.5.0
+
+    Returns
+    -------
+    Grid : Dataset
+        Dataset containing a CORDEX grid.
+
     """
+    if add_vertices is True:
+        warn(
+            "add_vertices keyword is deprecated, please use the bounds keyword instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        bounds = True
+
     rotated = True
     if attrs == "CORDEX":
         attrs = cf.DEFAULT_CORDEX_ATTRS
@@ -177,6 +220,9 @@ def create_dataset(
         attrs = {}
     if name:
         attrs["CORDEX_domain"] = name
+    # remove inconsistencies in keyword names
+    if short_name:
+        attrs["CORDEX_domain"] = short_name
     if pollon is None or pollat is None:
         rotated = False
     try:
@@ -188,27 +234,30 @@ def create_dataset(
     x, y = _lin_coord(nlon, dlon, ll_lon), _lin_coord(nlat, dlat, ll_lat)
 
     if rotated is True:
-        return _get_rotated_dataset(
+        ds = _get_rotated_dataset(
             x,
             y,
             #  lon,
             #  lat,
             pollon,
             pollat,
-            add_vertices=add_vertices,
-            dummy=dummy,
+            bounds=bounds,
             mapping_name=mapping_name,
             attrs=attrs,
         )
     else:
-        return _get_regular_dataset(
+        ds = _get_regular_dataset(
             x,
             y,
-            add_vertices=add_vertices,
-            dummy=dummy,
+            bounds=bounds,
             attrs=attrs,
         )
 
+    if dummy:
+        ds = _add_dummy(ds, dummy)
+
+    return ds
+
 
 def domain_info(short_name, tables=None):
     """Returns a dictionary containg the domain grid definitions.
@@ -241,8 +290,7 @@ def domain_info(short_name, tables=None):
 def _get_regular_dataset(
     lon,
     lat,
-    add_vertices=False,
-    dummy=None,
+    bounds=False,
     attrs=None,
 ):
     ds = xr.Dataset(
@@ -261,21 +309,9 @@ def _get_regular_dataset(
     ds.lon.attrs["axis"] = "X"
     ds.lat.attrs["axis"] = "Y"
 
-    if add_vertices is True:
-        from cartopy import crs as ccrs
-
-        pole = (
-            ds[mapping_name].grid_north_pole_longitude,
-            ds[mapping_name].grid_north_pole_latitude,
-        )
-        # v = vertices(ds, ccrs.RotatedPole(*pole))
-        v = vertices(ds.rlon, ds.rlat, ccrs.RotatedPole(*pole))
-        ds = xr.merge([ds, v])
-        ds[cf.LON_NAME].attrs["bounds"] = cf.LON_BOUNDS
-        ds[cf.LAT_NAME].attrs["bounds"] = cf.LAT_BOUNDS
+    if bounds is True:
+        ds = ds.cf.add_bounds(("lon", "lat"))
 
-    if dummy is not None:
-        ds = _add_dummy(ds, dummy)
     return ds
 
 
@@ -284,8 +320,7 @@ def _get_rotated_dataset(
     rlat,
     pollon,
     pollat,
-    add_vertices=False,
-    dummy=None,
+    bounds=False,
     mapping_name=None,
     attrs=None,
 ):
@@ -304,18 +339,16 @@ def _get_rotated_dataset(
     lon, lat = transform(ds.rlon, ds.rlat, src_crs=CRS.from_cf(mapping.attrs))
     ds = ds.assign_coords(lon=lon, lat=lat)
 
-    if add_vertices is True:
+    for key, coord in ds.coords.items():
+        coord.encoding["_FillValue"] = None
+        coord.attrs = cf.coords[key]
+
+    if bounds is True:
         v = vertices(ds.rlon, ds.rlat, CRS.from_cf(mapping.attrs))
         ds = xr.merge([ds, v])
         ds[cf.LON_NAME].attrs["bounds"] = cf.LON_BOUNDS
         ds[cf.LAT_NAME].attrs["bounds"] = cf.LAT_BOUNDS
 
-    for key, coord in ds.coords.items():
-        coord.encoding["_FillValue"] = None
-        coord.attrs = cf.coords[key]
-
-    if dummy is not None:
-        ds = _add_dummy(ds, dummy)
     return ds
 
 
diff --git a/cordex/transform.py b/cordex/transform.py
index 04f1301..4c5c55d 100644
--- a/cordex/transform.py
+++ b/cordex/transform.py
@@ -269,7 +269,7 @@ def grid_mapping(pollon, pollat, mapping_name=None):
     """creates a grid mapping DataArray object"""
     if mapping_name is None:
         mapping_name = cf.DEFAULT_MAPPING_NCVAR
-    da = xr.DataArray(np.zeros((), dtype=np.int32))
+    da = xr.DataArray(np.zeros((), dtype=cf.grid_mapping_dtype))
     attrs = cf.mapping.copy()
     attrs["grid_north_pole_longitude"] = pollon
     attrs["grid_north_pole_latitude"] = pollat
diff --git a/docs/whats_new.rst b/docs/whats_new.rst
index fa0b6f1..bfef787 100644
--- a/docs/whats_new.rst
+++ b/docs/whats_new.rst
@@ -18,6 +18,11 @@ New Features
 
 - Added :py:meth:`transform` and :py:meth:`transform_coords` in favour of deprecated :py:meth:`map_crs` and :py:meth:`rotated_coord_transform`. Dropped ``cartopy`` dependency in favour of ``pyproj`` (:pull:`71`).
 
+Deprecations
+~~~~~~~~~~~~
+
+- ``add_vertices`` keyword is deprecated in favour of ``bounds`` keyword in :py:meth:`cordex_domain` and :py:meth:`create_dataset` (:pull:`101`).
+
 Internal Changes
 ~~~~~~~~~~~~~~~~
 
@@ -27,6 +32,8 @@ Internal Changes
 - Moved core modules (:pull:`87`).
 - Added ``CITATION.cff`` (:pull:`87`).
 - Updated documentation environment for python 3.10 (:pull:`89`).
+- Use ``cf-xarray`` for adding bounds to regular datasets (:pull:`101`).
+
 
 
 v0.4.1 (23 June 2022)


Test patch:
diff --git a/tests/test_domain.py b/tests/test_domain.py
index b9d601c..d4abc54 100644
--- a/tests/test_domain.py
+++ b/tests/test_domain.py
@@ -1,34 +1,55 @@
 import numpy as np
+import pytest
+import xarray as xr
 
 import cordex as cx
 
 from . import requires_cartopy
 
 
-def test_domain_basic():
-    eur11 = cx.cordex_domain("EUR-11")
-    assert eur11.rlon.ndim == 1
-    assert eur11.rlat.ndim == 1
-    assert eur11.lon.ndim == 2
-    assert eur11.lat.ndim == 2
-    assert "lon_vertices" in cx.cordex_domain("EUR-11", add_vertices=True)
-    assert "lat_vertices" in cx.cordex_domain("EUR-11", add_vertices=True)
-    assert "rotated_pole" in cx.cordex_domain("EUR-11", mapping_name="rotated_pole")
-    assert "rotated_pole" in cx.cordex_domain("EUR-11", mapping_name="rotated_pole")
-    assert "dummy" in cx.cordex_domain("EUR-11", dummy=True)
-    assert "topo" in cx.cordex_domain("EUR-11", dummy="topo")
+@pytest.mark.parametrize("domain_id", ["EUR-11", "EUR-11i"])
+@pytest.mark.parametrize("bounds", [False, True])
+@pytest.mark.parametrize("mapping_name", [None, "rotated_pole"])
+@pytest.mark.parametrize("dummy", [False, True, "data", "topo"])
+def test_domain_coordinates(domain_id, bounds, mapping_name, dummy):
+    ds = cx.cordex_domain(
+        domain_id, bounds=bounds, mapping_name=mapping_name, dummy=dummy
+    )
+    assert ds.cf["X"].ndim == 1
+    assert ds.cf["Y"].ndim == 1
+
+    if ds.cf.grid_mapping_names:
+        assert ds.cf["longitude"].ndim == 2
+        assert ds.cf["latitude"].ndim == 2
+        if mapping_name:
+            assert [mapping_name] in ds.cf.grid_mapping_names.values()
+        else:
+            assert ["rotated_latitude_longitude"] in ds.cf.grid_mapping_names.values()
+    else:
+        assert ds.cf["longitude"].ndim == 1
+        assert ds.cf["latitude"].ndim == 1
+        assert ds.cf.grid_mapping_names == {}
+
+    if bounds is True:
+        assert "longitude" in ds.cf.bounds
+        assert "latitude" in ds.cf.bounds
+
+    if dummy is True:
+        assert "dummy" in ds
+    if isinstance(dummy, str):
+        assert dummy in ds
+
     assert cx.cordex_domain("EUR-11").attrs["CORDEX_domain"] == "EUR-11"
     assert "institution" in cx.cordex_domain("EUR-11", attrs="CORDEX").attrs
-    assert "rotated_latitude_longitude" not in cx.cordex_domain("EUR-11i").data_vars
-
-    # ensure rounding errors fixed
-    assert np.float64(eur11.rlon.isel(rlon=34).data) == -24.635
 
 
-def test_cordex_regular():
-    eur11i = cx.cordex_domain("EUR-11i")
-    assert eur11i.lon.ndim == 1
-    assert eur11i.lat.ndim == 1
+def test_domain():
+    ds = cx.cordex_domain("EUR-11")
+    ds.attrs["CORDEX_domain"] == "EUR-11"
+    # test attributes
+    assert "institution" in cx.cordex_domain("EUR-11", attrs="CORDEX").attrs
+    # ensure rounding errors fixed
+    assert np.float64(ds.rlon.isel(rlon=34).data) == -24.635
 
 
 def test_constructor():
@@ -46,7 +67,9 @@ def test_constructor():
     assert eur11_user.equals(eur11)
 
 
-def test_domain_info():
+@pytest.mark.parametrize("bounds", [False, True])
+# @pytest.mark.parametrize("", [2, 3])
+def test_domain_info(bounds):
     import pandas as pd
 
     info = {
@@ -66,6 +89,10 @@ def test_domain_info():
     }
     table = pd.DataFrame(info, index=[0]).set_index("short_name")
     assert cx.domain_info("EUR-11", table) == info
+    xr.testing.assert_equal(
+        cx.create_dataset(**info, bounds=bounds),
+        cx.cordex_domain("EUR-11", bounds=bounds),
+    )
 
 
 @requires_cartopy