Keep fill value for encoding (#369)

* Keep fill value for encoding * Add to release notes * Remove _FillValue from attributes in dmrpp _parse_variable * Slightly modify release notes * Pin icechunk for now and fix test * Pin zarr version to <2025.0.0 --------- Co-authored-by: Tom Nicholas <[email protected]>
zarr-developers · Jan 11, 2025 · 908ad94 · 908ad94
1 parent bd010c4
commit 908ad94
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 8 deletions.
diff --git a/ci/upstream.yml b/ci/upstream.yml
@@ -3,7 +3,7 @@ channels:
   - conda-forge
   - nodefaults
 dependencies:
-  - xarray>=2024.10.0
+  - xarray>=2024.10.0,<2025.0.0
   - h5netcdf
   - h5py
   - hdf5
@@ -28,6 +28,6 @@ dependencies:
   - fsspec
   - pip
   - pip:
-      - icechunk>=0.1.0a8 # Installs zarr v3 as dependency
+      - icechunk==0.1.0a8 # Installs zarr v3 beta 3 as dependency
       # - git+https://github.com/fsspec/kerchunk@main  # kerchunk is currently incompatible with zarr-python v3 (https://github.com/fsspec/kerchunk/pull/516)
       - imagecodecs-numcodecs==2024.6.1
diff --git a/docs/releases.rst b/docs/releases.rst
@@ -35,6 +35,8 @@ Bug fixes
 
 - Fix bug preventing generating references for the root group of a file when a subgroup exists.
   (:issue:`336`, :pull:`338`) By `Tom Nicholas <https://github.com/TomNicholas>`_.
+- Fix bug in dmrpp reader so _FillValue is included in variables' encodings.
+  (:pull:`369`) By `Aimee Barciauskas <https://github.com/abarciauskas-bgse>`_.
 - Fix bug passing arguments to FITS reader, and test it on Hubble Space Telescope data.
   (:pull:`363`) By `Tom Nicholas <https://github.com/TomNicholas>`_.
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -21,7 +21,7 @@ classifiers = [
 requires-python = ">=3.10"
 dynamic = ["version"]
 dependencies = [
-    "xarray>=2024.10.0",
+    "xarray>=2024.10.0,<2025.0.0",
     "numpy>=2.0.0",
     "packaging",
     "universal-pathlib",
@@ -39,7 +39,7 @@ hdf_reader = [
     "numcodecs"
 ]
 icechunk = [
-    "icechunk>=0.1.0a8",
+    "icechunk==0.1.0a8",
 ]
 test = [
     "codecov",

diff --git a/virtualizarr/readers/dmrpp.py b/virtualizarr/readers/dmrpp.py
@@ -411,7 +411,8 @@ def _parse_variable(self, var_tag: ET.Element) -> Variable:
         attrs: dict[str, Any] = {}
         for attr_tag in var_tag.iterfind("dap:Attribute", self._NS):
             attrs.update(self._parse_attribute(attr_tag))
-        # Fill value is placed in encoding and thus removed from attributes
+        # Fill value is placed in zarr array's fill_value and variable encoding and removed from attributes
+        encoding = {k: attrs.get(k) for k in self._ENCODING_KEYS if k in attrs}
         fill_value = attrs.pop("_FillValue", None)
         # create ManifestArray and ZArray
         zarray = ZArray(
@@ -423,7 +424,6 @@ def _parse_variable(self, var_tag: ET.Element) -> Variable:
             shape=shape,
         )
         marr = ManifestArray(zarray=zarray, chunkmanifest=chunkmanifest)
-        encoding = {k: attrs.get(k) for k in self._ENCODING_KEYS if k in attrs}
         return Variable(dims=dims.keys(), data=marr, attrs=attrs, encoding=encoding)
 
     def _parse_attribute(self, attr_tag: ET.Element) -> dict[str, Any]:

diff --git a/virtualizarr/tests/test_readers/test_dmrpp.py b/virtualizarr/tests/test_readers/test_dmrpp.py
@@ -296,7 +296,11 @@ def test_parse_variable(tmp_path):
     assert var.shape == (720, 1440)
     assert var.data.zarray.chunks == (360, 720)
     assert var.data.zarray.fill_value == -32768
-    assert var.encoding == {"add_offset": 298.15, "scale_factor": 0.001}
+    assert var.encoding == {
+        "add_offset": 298.15,
+        "scale_factor": 0.001,
+        "_FillValue": -32768,
+    }
     assert var.attrs == {
         "long_name": "analysed sea surface temperature",
         "items": [1, 2, 3],

diff --git a/virtualizarr/tests/test_writers/test_icechunk.py b/virtualizarr/tests/test_writers/test_icechunk.py
@@ -710,7 +710,9 @@ async def test_append_with_multiple_root_arrays(
                 "time/c/0", prototype=default_buffer_prototype()
             )
         ) == first_time_chunk_before_append
-        new_ds = open_zarr(icechunk_filestore_append, consolidated=False, zarr_format=3)
+        new_ds = open_zarr(
+            icechunk_filestore_append.store, consolidated=False, zarr_format=3
+        )
 
         expected_ds1, expected_ds2 = open_dataset(filepath1), open_dataset(filepath2)
         expected_ds = concat([expected_ds1, expected_ds2], dim="time")