Allow concatenating empty variables (#641)

* Allow concatenating empty variables

* Add test

* add test for indexing scalar with an ellipsis

* simplify test

* release note

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add note about test being a regression test

---------

Co-authored-by: Tom Nicholas <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

docs/releases.md

@@ -47,6 +47,8 @@
   By [Tom Nicholas](https://github.com/TomNicholas).
 - Fixed bug causing coordinates to be demoted to data variables when writing to Icechunk ([#574](https://github.com/zarr-developers/VirtualiZarr/issues/574), [#588](https://github.com/zarr-developers/VirtualiZarr/pull/588))
   By [Tom Nicholas](https://github.com/TomNicholas).
+- Fixed bug when indexing a scalar ManifestArray with an ellipsis([#596](https://github.com/zarr-developers/VirtualiZarr/issues/596), [#641](https://github.com/zarr-developers/VirtualiZarr/pull/641))
+  By [Max Jones](https://github.com/maxrjones) and [Tom Nicholas](https://github.com/TomNicholas).
 
 ### Documentation
 

virtualizarr/manifests/array.py

@@ -259,7 +259,7 @@ def __getitem__(
 
         # check value is valid
         indexer = _possibly_expand_trailing_ellipsis(indexer, self.ndim)
-        if len(indexer) != self.ndim:
+        if len(indexer) != self.ndim and self.ndim > 0:
             raise ValueError(
                 f"Invalid indexer for array. Indexer length must be less than or equal to the number of dimensions in the array, "
                 f"but indexer={indexer} has length {len(indexer)} and array has {self.ndim} dimensions."
@@ -362,7 +362,7 @@ def _possibly_expand_trailing_ellipsis(
     """
     final_dim_indexer = indexer[-1]
     if final_dim_indexer == ...:
-        if len(indexer) > ndim:
+        if len(indexer) > ndim and ndim > 0:
             raise ValueError(
                 f"Invalid indexer for array. Indexer length must be less than or equal to the number of dimensions in the array, "
                 f"but indexer={indexer} has length {len(indexer)} and array has {ndim} dimensions."

virtualizarr/tests/test_manifests/test_array.py

@@ -422,6 +422,11 @@ def test_index_with_numpy_array_notimplemented(self, manifest_array, dodgy_index
         with pytest.raises(NotImplementedError):
             marr[dodgy_indexer]
 
+    def test_indexing_scalar_with_ellipsis(self, manifest_array):
+        # regression test for https://github.com/zarr-developers/VirtualiZarr/pull/641
+        marr = manifest_array(shape=(), chunks=())
+        assert marr[...] == marr
+
 
 def test_to_xarray(array_v3_metadata):
     chunks = (5, 10)

virtualizarr/tests/test_xarray.py

@@ -878,3 +878,12 @@ def test_drop_variables(netcdf4_file):
         file_url=netcdf4_file, object_store=store, parser=parser, drop_variables=["air"]
     ) as vds:
         assert "air" not in vds.variables
+
+
+def test_concat_zero_dimensional_var(manifest_array):
+    # regression test for https://github.com/zarr-developers/VirtualiZarr/pull/641
+    marr = manifest_array(shape=(), chunks=())
+    vds1 = xr.Dataset({"a": marr})
+    vds2 = xr.Dataset({"a": marr})
+    result = xr.concat([vds1, vds2], dim="time", coords="minimal", compat="override")
+    assert result["a"].sizes == {"time": 2}