From 5cb3d218ade385fa5c2e500c967eb37258456d85 Mon Sep 17 00:00:00 2001
From: Matthew Iannucci <matthew@earthmover.io>
Date: Mon, 21 Oct 2024 16:13:04 -0400
Subject: [PATCH] Support loadables without tests

---
 .../tests/test_writers/test_icechunk.py         |  4 ++--
 virtualizarr/writers/icechunk.py                | 17 +++++++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/virtualizarr/tests/test_writers/test_icechunk.py b/virtualizarr/tests/test_writers/test_icechunk.py
index 401f1a57..52aea027 100644
--- a/virtualizarr/tests/test_writers/test_icechunk.py
+++ b/virtualizarr/tests/test_writers/test_icechunk.py
@@ -20,7 +20,7 @@
     from icechunk import IcechunkStore  # type: ignore[import-not-found]
 
 
-@pytest.fixture
+@pytest.fixture(scope="function")
 def icechunk_filestore(tmpdir) -> "IcechunkStore":
     from icechunk import IcechunkStore, StorageConfig
 
@@ -28,7 +28,7 @@ def icechunk_filestore(tmpdir) -> "IcechunkStore":
 
     # TODO if icechunk exposed a synchronous version of .open then we wouldn't need to use asyncio.run here
     # TODO is this the correct mode to use?
-    store = asyncio.run(IcechunkStore.open(storage=storage, mode="a"))
+    store = IcechunkStore.create(storage=storage, mode="w")
 
     # TODO instead yield store then store.close() ??
     return store
diff --git a/virtualizarr/writers/icechunk.py b/virtualizarr/writers/icechunk.py
index 96adfaf9..468de988 100644
--- a/virtualizarr/writers/icechunk.py
+++ b/virtualizarr/writers/icechunk.py
@@ -64,14 +64,22 @@ def write_variables_to_icechunk_group(
     store,
     group,
 ):
-    for name, var in variables.items():
-        write_variable_to_icechunk(
+    print(variables)
+    virtual_variables = {name: var for name, var in variables.items() if isinstance(var.data, ManifestArray)}
+
+    for name, var in virtual_variables.items():
+        write_virtual_variable_to_icechunk(
             store=store,
             group=group,
             name=name,
             var=var,
         )
 
+    loadable_variables = {name: var for name, var in variables.items() if name not in virtual_variables}
+    
+    ds = Dataset(loadable_variables)
+    ds.to_zarr(store, zarr_format=3, consolidated=False, mode="r+")
+
 
 def write_variable_to_icechunk(
     store: "IcechunkStore",
@@ -80,7 +88,6 @@ def write_variable_to_icechunk(
     var: Variable,
 ) -> None:
     """Write a single (possibly virtual) variable into an icechunk store"""
-
     if isinstance(var.data, ManifestArray):
         write_virtual_variable_to_icechunk(
             store=store,
@@ -89,9 +96,7 @@ def write_variable_to_icechunk(
             var=var,
         )
     else:
-        # TODO is writing loadable_variables just normal xarray ds.to_zarr?
-        # raise NotImplementedError()
-        print("skipping non-virtual variable", name)
+        raise ValueError("Cannot write non-virtual variables as virtual variables to Icechunk stores")
 
 
 def write_virtual_variable_to_icechunk(