Repository: zarr-developers/VirtualiZarr

Issue:
NetCDF files with many empty variables require large `drop_variables` declaration
I'm not sure if this is a real issue or working as intended, but I wanted to get clarification on it. The [GOES](https://planetarycomputer.microsoft.com/dataset/goes-cmi) dataset's NetCDF files contain many non-array variables such that the call to `virtualizarr.open_virtual_dataset` needs a large number of `drop_variables` to work. All of the dropped variables have in common a `chunk_shape` of `()`, because they're not arrays.

If [`validate_chunk_shape`](https://github.com/zarr-developers/VirtualiZarr/blob/5d0851976778ebaf52eb6df57970b0706a892d98/virtualizarr/manifests/manifest.py#L360) skipped validating empty dicts, it would be possible to open this dataset without having to pass such a `drop_variables` list, but more importantly we could preserve variable metadata when we write a Zarr store for this file and others. Is there a reason VirtualiZarr shouldn't read empty variables like this?

## Repro
```python
import planetary_computer
import pystac_client
import virtualizarr as vz

catalog = pystac_client.Client.open("https://planetarycomputer.microsoft.com/api/stac/v1/")
search = catalog.search(
    collections=["goes-cmi"],
    datetime=["2019-08-01T12:00:00Z", "2019-08-01T13:00:00Z"],
    query={"goes:image-type": {"eq": "FULL DISK"}},
)
items = sorted(search.item_collection(), key=lambda x: x.datetime)
item = items[0]
vds = vz.open_virtual_dataset(
    planetary_computer.sign(item.assets["CMIP_C15-nc"].href),
    # drop_variables=[
    #     "processing_parm_version_container",
    #     "earth_sun_distance_anomaly_in_AU",
    #     "goes_imager_projection",
    #     "esun",
    #     "algorithm_product_version_container",
    #     "std_dev_brightness_temperature",
    #     "maximum_focal_plane_temperature",
    #     "focal_plane_temperature_threshold_increasing",
    #     "focal_plane_temperature_threshold_exceeded_count",
    #     "y_image",
    #     "nominal_satellite_height",
    #     "outlier_pixel_count",
    #     "focal_plane_temperature_threshold_decreasing",
    #     "min_brightness_temperature",
    #     "planck_bc2",
    #     "nominal_satellite_subpoint_lon",
    #     "nominal_satellite_subpoint_lat",
    #     "valid_pixel_count",
    #     "planck_bc1",
    #     "mean_brightness_temperature",
    #     "planck_fk2",
    #     "percent_uncorrectable_GRB_errors",
    #     "geospatial_lat_lon_extent",
    #     "kappa0",
    #     "percent_uncorrectable_L0_errors",
    #     "t",
    #     "max_brightness_temperature",
    #     "x_image",
    #     "planck_fk1",
    #     "algorithm_dynamic_input_data_container",
    #     "total_number_of_points"],
    reader_options={},
)
```


Discussion:
sharkinsspatial: @ghidalgo3 Here is an HDF5 fixture you could use for direct testing of this behavior without mocking.  
ghidalgo3: Thanks @sharkinsspatial for the fixtures! I added test cases for both cases without that ugly mock. @TomNicholas if it looks good to you, it's ready to merge.
ghidalgo3: I'm running into the problem described by #62 but I think I don't want to solve it in this PR.

You can open files with scalar variables, and empty variables, but if you call `vds.virtualize.to_zarr("store.zarr")` then that will error because one of the variables is a numpy array and not a `ManifestArray`. Thats the behavior today anyway, and should be solved once #62 is implemented. If you truly must write the `vds` to disk, just skip loading the variables with `drop_variables`.

Given that, this PR is good to merge.

Patch:
diff --git a/docs/releases.rst b/docs/releases.rst
index f472db0..81a0aea 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -9,6 +9,9 @@ v1.0.1 (unreleased)
 New Features
 ~~~~~~~~~~~~
 
+- Load scalar variables by default. (:pull:`205`)
+  By `Gustavo Hidalgo <https://github.com/ghidalgo3>`_.
+
 Breaking changes
 ~~~~~~~~~~~~~~~~
 
diff --git a/pyproject.toml b/pyproject.toml
index 9fe0468..4496116 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -45,6 +45,7 @@ test = [
     "fsspec",
     "s3fs",
     "fastparquet",
+    "h5py"
 ]
 
 
diff --git a/virtualizarr/xarray.py b/virtualizarr/xarray.py
index 3a6dd02..528c252 100644
--- a/virtualizarr/xarray.py
+++ b/virtualizarr/xarray.py
@@ -333,13 +333,16 @@ def variable_from_kerchunk_refs(
 
     arr_refs = kerchunk.extract_array_refs(refs, var_name)
     chunk_dict, zarray, zattrs = kerchunk.parse_array_refs(arr_refs)
-
-    manifest = ChunkManifest._from_kerchunk_chunk_dict(chunk_dict)
-
     # we want to remove the _ARRAY_DIMENSIONS from the final variables' .attrs
     dims = zattrs.pop("_ARRAY_DIMENSIONS")
-
-    varr = virtual_array_class(zarray=zarray, chunkmanifest=manifest)
+    if chunk_dict:
+        manifest = ChunkManifest._from_kerchunk_chunk_dict(chunk_dict)
+        varr = virtual_array_class(zarray=zarray, chunkmanifest=manifest)
+    else:
+        # This means we encountered a scalar variable of dimension 0,
+        # very likely that it actually has no numeric value and its only purpose
+        # is to communicate dataset attributes.
+        varr = zarray.fill_value
 
     return xr.Variable(data=varr, dims=dims, attrs=zattrs)
 


Test patch:
diff --git a/conftest.py b/conftest.py
index 8d5e351..b558abf 100644
--- a/conftest.py
+++ b/conftest.py
@@ -1,3 +1,4 @@
+import h5py
 import pytest
 import xarray as xr
 
@@ -50,3 +51,21 @@ def netcdf4_files(tmpdir):
     ds2.close()
 
     return filepath1, filepath2
+
+
+@pytest.fixture
+def hdf5_empty(tmpdir):
+    filepath = f"{tmpdir}/empty.nc"
+    f = h5py.File(filepath, "w")
+    dataset = f.create_dataset("empty", shape=(), dtype="float32")
+    dataset.attrs["empty"] = "true"
+    return filepath
+
+
+@pytest.fixture
+def hdf5_scalar(tmpdir):
+    filepath = f"{tmpdir}/scalar.nc"
+    f = h5py.File(filepath, "w")
+    dataset = f.create_dataset("scalar", data=0.1, dtype="float32")
+    dataset.attrs["scalar"] = "true"
+    return filepath
diff --git a/virtualizarr/tests/test_xarray.py b/virtualizarr/tests/test_xarray.py
index 31ef832..00140a1 100644
--- a/virtualizarr/tests/test_xarray.py
+++ b/virtualizarr/tests/test_xarray.py
@@ -419,6 +419,16 @@ class TestLoadVirtualDataset:
         }
         mock_read_kerchunk.assert_called_once_with(**args)
 
+    def test_open_dataset_with_empty(self, hdf5_empty, tmpdir):
+        vds = open_virtual_dataset(hdf5_empty)
+        assert vds.empty.dims == ()
+        assert vds.empty.attrs == {"empty": "true"}
+
+    def test_open_dataset_with_scalar(self, hdf5_scalar, tmpdir):
+        vds = open_virtual_dataset(hdf5_scalar)
+        assert vds.scalar.dims == ()
+        assert vds.scalar.attrs == {"scalar": "true"}
+
 
 class TestRenamePaths:
     def test_rename_to_str(self, netcdf4_file):