Repository: h5netcdf/h5netcdf (language: python)

Issue:
Support _FillValue for variable length strings
Current status:
- We can read them, for files written with netCDF4-python (via https://github.com/Unidata/netcdf4-python/pull/732).
- We can't write them, due to https://github.com/h5py/h5py/issues/941.


Discussion / hints:
Update: https://github.com/h5py/h5py/issues/941#issuecomment-1029756453
Good news -> Possible fix in https://github.com/h5py/h5py/pull/2044
https://github.com/h5py/h5py/pull/2044 is merged and available in h5py 3.7

Fix patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index ae23471a..85a8d56c 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,5 +1,10 @@
 Change Log
 ----------
+Development Version:
+
+- Rework adding _FillValue-attribute, add tests.
+  By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.
+
 Version 1.0.2 (August 2nd, 2022):
 
 - Adapt boolean indexing as h5py 3.7.0 started supporting it.
diff --git a/h5netcdf/core.py b/h5netcdf/core.py
index 6716dfec..32902152 100644
--- a/h5netcdf/core.py
+++ b/h5netcdf/core.py
@@ -712,7 +712,20 @@ def _create_child_variable(
         variable._ensure_dim_id()
 
         if fillvalue is not None:
-            value = variable.dtype.type(fillvalue)
+            # trying to create correct type of fillvalue
+            if variable.dtype is str:
+                value = fillvalue
+            else:
+                string_info = self._root._h5py.check_string_dtype(variable.dtype)
+                if (
+                    string_info
+                    and string_info.length is not None
+                    and string_info.length > 1
+                ):
+                    value = fillvalue
+                else:
+                    value = variable.dtype.type(fillvalue)
+
             variable.attrs._h5attrs["_FillValue"] = value
         return variable
 


Test patch:
diff --git a/h5netcdf/tests/test_h5netcdf.py b/h5netcdf/tests/test_h5netcdf.py
index 392d8ca8..2cdc749a 100644
--- a/h5netcdf/tests/test_h5netcdf.py
+++ b/h5netcdf/tests/test_h5netcdf.py
@@ -2025,3 +2025,78 @@ def test_array_attributes(tmp_local_netcdf):
         np.testing.assert_equal(ds.int_array, np.arange(10))
         np.testing.assert_equal(ds.empty_list, np.array([]))
         np.testing.assert_equal(ds.empty_array, np.array([]))
+
+
+@pytest.mark.skipif(
+    version.parse(h5py.__version__) < version.parse("3.7.0"),
+    reason="does not work with h5py < 3.7.0",
+)
+def test_vlen_string_dataset_fillvalue(tmp_local_netcdf, decode_vlen_strings):
+    # check _FillValue for VLEN string datasets
+    # only works for h5py >= 3.7.0
+
+    # first with new API
+    with h5netcdf.File(tmp_local_netcdf, "w") as ds:
+        ds.dimensions = {"string": 10}
+        dt0 = h5py.string_dtype()
+        fill_value0 = "bár"
+        ds.create_variable("x0", ("string",), dtype=dt0, fillvalue=fill_value0)
+        dt1 = h5py.string_dtype("ascii")
+        fill_value1 = "bar"
+        ds.create_variable("x1", ("string",), dtype=dt1, fillvalue=fill_value1)
+
+    # check, if new API can read them
+    with h5netcdf.File(tmp_local_netcdf, "r", **decode_vlen_strings) as ds:
+        decode_vlen = decode_vlen_strings["decode_vlen_strings"]
+        fvalue0 = fill_value0 if decode_vlen else fill_value0.encode("utf-8")
+        fvalue1 = fill_value1 if decode_vlen else fill_value1.encode("utf-8")
+        assert ds["x0"][0] == fvalue0
+        assert ds["x0"].attrs["_FillValue"] == fill_value0
+        assert ds["x1"][0] == fvalue1
+        assert ds["x1"].attrs["_FillValue"] == fill_value1
+
+    # check if legacyapi can read them
+    with legacyapi.Dataset(tmp_local_netcdf, "r") as ds:
+        assert ds["x0"][0] == fill_value0
+        assert ds["x0"]._FillValue == fill_value0
+        assert ds["x1"][0] == fill_value1
+        assert ds["x1"]._FillValue == fill_value1
+
+    # check if netCDF4-python can read them
+    with netCDF4.Dataset(tmp_local_netcdf, "r") as ds:
+        assert ds["x0"][0] == fill_value0
+        assert ds["x0"]._FillValue == fill_value0
+        assert ds["x1"][0] == fill_value1
+        assert ds["x1"]._FillValue == fill_value1
+
+    # second with legacyapi
+    with legacyapi.Dataset(tmp_local_netcdf, "w") as ds:
+        ds.createDimension("string", 10)
+        fill_value0 = "bár"
+        ds.createVariable("x0", str, ("string",), fill_value=fill_value0)
+        fill_value1 = "bar"
+        ds.createVariable("x1", str, ("string",), fill_value=fill_value1)
+
+    # check if new API can read them
+    with h5netcdf.File(tmp_local_netcdf, "r", **decode_vlen_strings) as ds:
+        decode_vlen = decode_vlen_strings["decode_vlen_strings"]
+        fvalue0 = fill_value0 if decode_vlen else fill_value0.encode("utf-8")
+        fvalue1 = fill_value1 if decode_vlen else fill_value1.encode("utf-8")
+        assert ds["x0"][0] == fvalue0
+        assert ds["x0"].attrs["_FillValue"] == fill_value0
+        assert ds["x1"][0] == fvalue1
+        assert ds["x1"].attrs["_FillValue"] == fill_value1
+
+    # check if legacyapi can read them
+    with legacyapi.Dataset(tmp_local_netcdf, "r") as ds:
+        assert ds["x0"][0] == fill_value0
+        assert ds["x0"]._FillValue == fill_value0
+        assert ds["x1"][0] == fill_value1
+        assert ds["x1"]._FillValue == fill_value1
+
+    # check if netCDF4-python can read them
+    with netCDF4.Dataset(tmp_local_netcdf, "r") as ds:
+        assert ds["x0"][0] == fill_value0
+        assert ds["x0"]._FillValue == fill_value0
+        assert ds["x1"][0] == fill_value1
+        assert ds["x1"]._FillValue == fill_value1