Repository: zarr-developers/VirtualiZarr

Issue:
Bugs with preserving variable-level attrs
There are some bugs with preserving variable-level attributes.

You can see this in #154, where [changing `assert_equal` to `assert_identical`](https://github.com/zarr-developers/VirtualiZarr/pull/154/commits/8ddac726a26294c195ba3cc53aa306fd0d857f0d) in the roundtripping tests in `test_integration.py` reveals that the attributes of the `lat` and `lon` coordinate variables [are being dropped](https://github.com/zarr-developers/VirtualiZarr/actions/runs/9635897427/job/26573199921?pr=154) accidentally.

Discussion:

Patch:
diff --git a/docs/releases.rst b/docs/releases.rst
index e7483c0..d41ae20 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -25,7 +25,9 @@ Deprecations
 Bug fixes
 ~~~~~~~~~
 
-- Ensure that `_ARRAY_DIMENSIONS` are dropped from variable `.attrs`. (:issue:`150`, :pull:`152`) 
+- Ensure that `_ARRAY_DIMENSIONS` are dropped from variable `.attrs`. (:issue:`150`, :pull:`152`)
+  By `Tom Nicholas <https://github.com/TomNicholas>`_.
+- Ensure that `.attrs` on coordinate variables are preserved during round-tripping. (:issue:`155`, :pull:`154`)
   By `Tom Nicholas <https://github.com/TomNicholas>`_.
 
 Documentation
diff --git a/virtualizarr/xarray.py b/virtualizarr/xarray.py
index 96dbf23..85f27c0 100644
--- a/virtualizarr/xarray.py
+++ b/virtualizarr/xarray.py
@@ -243,7 +243,6 @@ def virtual_vars_from_kerchunk_refs(
         var_name: variable_from_kerchunk_refs(refs, var_name, virtual_array_class)
         for var_name in var_names_to_keep
     }
-
     return vars
 
 
@@ -306,7 +305,7 @@ def separate_coords(
     """
     Try to generate a set of coordinates that won't cause xarray to automatically build a pandas.Index for the 1D coordinates.
 
-    Currently requires a workaround unless xarray 8107 is merged.
+    Currently requires this function as a workaround unless xarray PR #8124 is merged.
 
     Will also preserve any loaded variables and indexes it is passed.
     """
@@ -322,7 +321,7 @@ def separate_coords(
             # use workaround to avoid creating IndexVariables described here https://github.com/pydata/xarray/pull/8107#discussion_r1311214263
             if len(var.dims) == 1:
                 dim1d, *_ = var.dims
-                coord_vars[name] = (dim1d, var.data)
+                coord_vars[name] = (dim1d, var.data, var.attrs)
 
                 if isinstance(var, IndexVariable):
                     # unless variable actually already is a loaded IndexVariable,


Test patch:
diff --git a/virtualizarr/tests/test_integration.py b/virtualizarr/tests/test_integration.py
index 064968b..50c0d64 100644
--- a/virtualizarr/tests/test_integration.py
+++ b/virtualizarr/tests/test_integration.py
@@ -55,14 +55,16 @@ class TestKerchunkRoundtrip:
         # use open_dataset_via_kerchunk to read it as references
         vds = open_virtual_dataset(f"{tmpdir}/air.nc", indexes={})
 
-        # write those references to disk as kerchunk json
+        # write those references to disk as kerchunk references format
         vds.virtualize.to_kerchunk(f"{tmpdir}/refs.{format}", format=format)
 
-        # use fsspec to read the dataset from disk via the zarr store
-        roundtrip = xr.open_dataset(f"{tmpdir}/refs.{format}", engine="kerchunk")
+        # use fsspec to read the dataset from disk via the kerchunk references
+        roundtrip = xr.open_dataset(
+            f"{tmpdir}/refs.{format}", engine="kerchunk", decode_times=False
+        )
 
-        # assert equal to original dataset
-        xrt.assert_equal(roundtrip, ds)
+        # assert identical to original dataset
+        xrt.assert_identical(roundtrip, ds)
 
     def test_kerchunk_roundtrip_concat(self, tmpdir, format):
         # set up example xarray dataset
@@ -82,14 +84,16 @@ class TestKerchunkRoundtrip:
         # concatenate virtually along time
         vds = xr.concat([vds1, vds2], dim="time", coords="minimal", compat="override")
 
-        # write those references to disk as kerchunk json
+        # write those references to disk as kerchunk references format
         vds.virtualize.to_kerchunk(f"{tmpdir}/refs.{format}", format=format)
 
-        # use fsspec to read the dataset from disk via the zarr store
-        roundtrip = xr.open_dataset(f"{tmpdir}/refs.{format}", engine="kerchunk")
+        # use fsspec to read the dataset from disk via the kerchunk references
+        roundtrip = xr.open_dataset(
+            f"{tmpdir}/refs.{format}", engine="kerchunk", decode_times=False
+        )
 
-        # assert equal to original dataset
-        xrt.assert_equal(roundtrip, ds)
+        # assert identical to original dataset
+        xrt.assert_identical(roundtrip, ds)
 
 
 def test_open_scalar_variable(tmpdir):
diff --git a/virtualizarr/tests/test_xarray.py b/virtualizarr/tests/test_xarray.py
index 447d920..e55583b 100644
--- a/virtualizarr/tests/test_xarray.py
+++ b/virtualizarr/tests/test_xarray.py
@@ -234,6 +234,16 @@ class TestOpenVirtualDatasetAttrs:
         vds = open_virtual_dataset(netcdf4_file, indexes={})
         assert "_ARRAY_DIMENSIONS" not in vds["air"].attrs
 
+    def test_coordinate_variable_attrs_preserved(self, netcdf4_file):
+        # regression test for GH issue #155
+        vds = open_virtual_dataset(netcdf4_file, indexes={})
+        assert vds["lat"].attrs == {
+            "standard_name": "latitude",
+            "long_name": "Latitude",
+            "units": "degrees_north",
+            "axis": "Y",
+        }
+
 
 class TestOpenVirtualDatasetIndexes:
     def test_no_indexes(self, netcdf4_file):